Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Kurt Kanzenbach <kurt@linutronix.de>
Cc: Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Richard Cochran <richardcochran@gmail.com>,
	Kamil Alkhouri <kamil.alkhouri@hs-offenburg.de>,
	ilias.apalodimas@linaro.org, Vladimir Oltean <olteanv@gmail.com>
Subject: Re: [PATCH v3 2/8] net: dsa: Add DSA driver for Hirschmann Hellcreek switches
Date: Tue, 25 Aug 2020 15:56:15 +0200	[thread overview]
Message-ID: <20200825135615.GR2588906@lunn.ch> (raw)
In-Reply-To: <87eenv14bt.fsf@kurt>

> >> +static int hellcreek_detect(struct hellcreek *hellcreek)
> >> +{
> >> +	u16 id, rel_low, rel_high, date_low, date_high, tgd_ver;
> >> +	u8 tgd_maj, tgd_min;
> >> +	u32 rel, date;
> >> +
> >> +	id	  = hellcreek_read(hellcreek, HR_MODID_C);
> >> +	rel_low	  = hellcreek_read(hellcreek, HR_REL_L_C);
> >> +	rel_high  = hellcreek_read(hellcreek, HR_REL_H_C);
> >> +	date_low  = hellcreek_read(hellcreek, HR_BLD_L_C);
> >> +	date_high = hellcreek_read(hellcreek, HR_BLD_H_C);
> >> +	tgd_ver   = hellcreek_read(hellcreek, TR_TGDVER);
> >> +
> >> +	if (id != HELLCREEK_MODULE_ID)
> >> +		return -ENODEV;
> >
> > Are there other Hellcreek devices? I'm just wondering if we should
> > have a specific compatible for 0x4c30 as well as the more generic 
> > "hirschmann,hellcreek".
> 
> Yes, there will be different revisions of the Hellcreek devices. This ID
> is really device specific. A lot of features of this switch are
> configured in the VHDL code. For instance the MAC settings (100 or 1000
> Mbit/s).
> 
> I've discussed this with the HW engineer from Hirschmann. He suggested
> to keep this check here, as the driver is currently specific for the
> that device. We have to make sure that the driver matches the hardware.

I agree with the check here. The question is about the compatible
string. Should there be a more specific compatible string as well as
the generic one?

There have been a few discussions about how the Marvell DSA driver
does its compatible string. The compatible string tells you where to
find the ID register, not what value to expect in the ID register. The
ID register can currently be in one of three different locations. Do
all current and future Hellcreak devices have the same value for
HR_MODID_C? If not, now is a good time to add a more specific
compatible string to tell you where to find the ID register.

> My plan was to extend this when I have access to other revisions. There
> will be a SPI variant as well. But, I didn't want to implement it without the
> ability to test it.

Does the SPI variant use the same value for HR_MODID_C? Maybe you need
a different compatible, maybe not, depending on how the driver is
structured.

The compatible string is part of the ABI. So thinking about it a bit
now can make things easier later. I just want to make sure you have
thought about this.

	Andrew

  reply	other threads:[~2020-08-25 13:56 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-20  8:11 [PATCH v3 0/8] Hirschmann Hellcreek DSA driver Kurt Kanzenbach
2020-08-20  8:11 ` [PATCH v3 1/8] net: dsa: Add tag handling for Hirschmann Hellcreek switches Kurt Kanzenbach
2020-08-20  8:11 ` [PATCH v3 2/8] net: dsa: Add DSA driver " Kurt Kanzenbach
2020-08-24 22:44   ` Andrew Lunn
2020-08-25  9:07     ` Kurt Kanzenbach
2020-08-25 13:56       ` Andrew Lunn [this message]
2020-08-25 14:48         ` Kurt Kanzenbach
2020-08-27 10:29           ` Kurt Kanzenbach
2020-08-20  8:11 ` [PATCH v3 3/8] net: dsa: hellcreek: Add PTP clock support Kurt Kanzenbach
2020-08-20  8:11 ` [PATCH v3 4/8] net: dsa: hellcreek: Add support for hardware timestamping Kurt Kanzenbach
2020-08-20  8:11 ` [PATCH v3 5/8] net: dsa: hellcreek: Add TAPRIO offloading support Kurt Kanzenbach
2020-08-22 14:39   ` Vladimir Oltean
2020-08-24  6:10     ` Kurt Kanzenbach
2020-08-24 23:45       ` Vinicius Costa Gomes
2020-08-25  9:42         ` Kurt Kanzenbach
2020-08-25 17:58           ` Vinicius Costa Gomes
2020-08-27 10:12             ` Kurt Kanzenbach
2020-08-25  9:46         ` Vladimir Oltean
2020-08-25 10:09           ` Kurt Kanzenbach
2020-08-25 17:33           ` Vinicius Costa Gomes
2020-08-24 22:56   ` Vladimir Oltean
2020-08-25  9:33     ` Kurt Kanzenbach
2020-08-25  9:38       ` Vladimir Oltean
2020-08-25  9:55         ` Kurt Kanzenbach
2020-08-27 16:25           ` Richard Cochran
2020-08-28 12:31             ` Kurt Kanzenbach
2020-08-24 23:57   ` Vinicius Costa Gomes
2020-08-25  9:23     ` Kurt Kanzenbach
2020-08-25  9:32       ` Vladimir Oltean
2020-09-01 14:20         ` Kurt Kanzenbach
2020-09-01 14:47           ` Vladimir Oltean
2020-08-25 17:50       ` Vinicius Costa Gomes
2020-08-20  8:11 ` [PATCH v3 6/8] net: dsa: hellcreek: Add PTP status LEDs Kurt Kanzenbach
2020-08-24 22:50   ` Andrew Lunn
2020-08-20  8:11 ` [PATCH v3 7/8] dt-bindings: Add vendor prefix for Hirschmann Kurt Kanzenbach
2020-08-20  8:11 ` [PATCH v3 8/8] dt-bindings: net: dsa: Add documentation for Hellcreek switches Kurt Kanzenbach
2020-08-24 22:52   ` Andrew Lunn
2020-08-25 22:28   ` Rob Herring
2020-08-24 21:31 ` [PATCH v3 0/8] Hirschmann Hellcreek DSA driver Jakub Kicinski
2020-08-24 22:02   ` Vladimir Oltean
2020-08-24 22:35     ` David Miller
2020-08-24 22:57       ` Vladimir Oltean
2020-08-25 11:21       ` Kurt Kanzenbach
2020-08-25 17:14         ` Florian Fainelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200825135615.GR2588906@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=bigeasy@linutronix.de \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=ilias.apalodimas@linaro.org \
    --cc=kamil.alkhouri@hs-offenburg.de \
    --cc=kuba@kernel.org \
    --cc=kurt@linutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=richardcochran@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=vivien.didelot@gmail.com \
    --subject='Re: [PATCH v3 2/8] net: dsa: Add DSA driver for Hirschmann Hellcreek switches' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox