| Message ID | 1404142889-20740-1-git-send-email-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
Hi, On Mon, Jun 30, 2014 at 8:41 AM, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > Now that both spi and serial driver support these (commits f2bb31057a42 > (spi: efm32: properly namespace location property) and > 74be65a3cff5 (serial: efm32: properly namespace location property)) use > the better names. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > arch/arm/boot/dts/efm32gg-dk3750.dts | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/boot/dts/efm32gg-dk3750.dts b/arch/arm/boot/dts/efm32gg-dk3750.dts > index b4031fa4a567..d5dd2a2a7970 100644 > --- a/arch/arm/boot/dts/efm32gg-dk3750.dts > +++ b/arch/arm/boot/dts/efm32gg-dk3750.dts > @@ -43,7 +43,7 @@ > > spi0: spi@4000c000 { /* USART0 */ > cs-gpios = <&gpio 68 1>; // E4 > - location = <1>; > + efm32,location = <1>; Hrm, the prefix is normally the vendor, not the platform. I see that the bindings have already been merged though. Sorry, I completely missed that the first time around. :( Probably not a whole lot to do about it in this case. It's not the end of the world anyway, efm32 is a sufficiently unique string as it is. -Olof
Hi Olof, On Mon, Jul 07, 2014 at 10:46:52PM -0700, Olof Johansson wrote: > On Mon, Jun 30, 2014 at 8:41 AM, Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > Now that both spi and serial driver support these (commits f2bb31057a42 > > (spi: efm32: properly namespace location property) and > > 74be65a3cff5 (serial: efm32: properly namespace location property)) use > > the better names. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > arch/arm/boot/dts/efm32gg-dk3750.dts | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm/boot/dts/efm32gg-dk3750.dts b/arch/arm/boot/dts/efm32gg-dk3750.dts > > index b4031fa4a567..d5dd2a2a7970 100644 > > --- a/arch/arm/boot/dts/efm32gg-dk3750.dts > > +++ b/arch/arm/boot/dts/efm32gg-dk3750.dts > > @@ -43,7 +43,7 @@ > > > > spi0: spi@4000c000 { /* USART0 */ > > cs-gpios = <&gpio 68 1>; // E4 > > - location = <1>; > > + efm32,location = <1>; > > Hrm, the prefix is normally the vendor, not the platform. I see that Don't considering what it used normally (and I didn't do it wrong on purpose) I consider a string identifying the platform to be more sensible here. For one thing because the vendor can change (as it did with efm32). Another reason is that the vendor can create another platform that also needs some devices with a location property that has a completely different semantic but would get the same name. (Ok, if the semantic on the different platform is similar $vendor,location would be more appropriate. Probably the decision on what name to pick should be considered case by case.) Just my 0.02 €, Uwe
On Mon, Jul 7, 2014 at 11:41 PM, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > Hi Olof, > > On Mon, Jul 07, 2014 at 10:46:52PM -0700, Olof Johansson wrote: >> On Mon, Jun 30, 2014 at 8:41 AM, Uwe Kleine-König >> <u.kleine-koenig@pengutronix.de> wrote: >> > Now that both spi and serial driver support these (commits f2bb31057a42 >> > (spi: efm32: properly namespace location property) and >> > 74be65a3cff5 (serial: efm32: properly namespace location property)) use >> > the better names. >> > >> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> >> > --- >> > arch/arm/boot/dts/efm32gg-dk3750.dts | 6 +++--- >> > 1 file changed, 3 insertions(+), 3 deletions(-) >> > >> > diff --git a/arch/arm/boot/dts/efm32gg-dk3750.dts b/arch/arm/boot/dts/efm32gg-dk3750.dts >> > index b4031fa4a567..d5dd2a2a7970 100644 >> > --- a/arch/arm/boot/dts/efm32gg-dk3750.dts >> > +++ b/arch/arm/boot/dts/efm32gg-dk3750.dts >> > @@ -43,7 +43,7 @@ >> > >> > spi0: spi@4000c000 { /* USART0 */ >> > cs-gpios = <&gpio 68 1>; // E4 >> > - location = <1>; >> > + efm32,location = <1>; >> >> Hrm, the prefix is normally the vendor, not the platform. I see that > Don't considering what it used normally (and I didn't do it wrong on > purpose) I consider a string identifying the platform to be more > sensible here. It's long-standing practice (20 years or so) to always use the vendor as prefix, not the platform. > For one thing because the vendor can change (as it did with > efm32). Then we keep the previous vendor, that's a long-standing practice too. Sun didn't switch from SUNW to JAVA as prefix just because their stock ticker was renamed. > Another reason is that the vendor can create another platform > that also needs some devices with a location property that has a > completely different semantic but would get the same name. Then the new binding will use a different name, or they will be managed in some compatible way. The same argument could be said in the other way: the same vendor might introduce a new platform that uses the same binding but isn't called efm32. That's equally wrong (and it's actually a quite common situation these days). > (Ok, if the semantic on the different platform is similar > $vendor,location would be more appropriate. Probably the decision on > what name to pick should be considered case by case.) I really don't want a hodge-podge of whatever someone felt like picking as a name at the time. It makes it really hard for newcomers to find a good best practices to follow, and over time will result in total chaos. What's this binding for anyway? It looks a lot like an ad-hoc pinctrl binding for configuring pinout, is that accurate? -Olof
Hi Olof, On Tue, Jul 08, 2014 at 09:06:13AM -0700, Olof Johansson wrote: > I really don't want a hodge-podge of whatever someone felt like > picking as a name at the time. It makes it really hard for newcomers > to find a good best practices to follow, and over time will result in > total chaos. That's right. I did it in good faith. If you think it would be worth to switch to say "energymicro,location" I think it wouldn't be that hard, because I think there are no boards in the wild that use a device tree and don't use the BSP I'm providing where switching is a no brainer. > > What's this binding for anyway? It looks a lot like an ad-hoc pinctrl > binding for configuring pinout, is that accurate? Well, it has to do with pinctrl, but it was agreed that managing it with a single pinctrl device would complicate things considerably and in the end the device tree wouldn't describe how the hardware looks like but would result in a lot of complexity just to fit into the Linux pinctrl model. See the thread around http://marc.info/?l=linux-spi&m=137485291802157&w=2 for the glory details. Best regards Uwe
On Tue, Jul 8, 2014 at 11:26 AM, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > Hi Olof, > > On Tue, Jul 08, 2014 at 09:06:13AM -0700, Olof Johansson wrote: >> I really don't want a hodge-podge of whatever someone felt like >> picking as a name at the time. It makes it really hard for newcomers >> to find a good best practices to follow, and over time will result in >> total chaos. > That's right. I did it in good faith. If you think it would be worth to > switch to say "energymicro,location" I think it wouldn't be that hard, > because I think there are no boards in the wild that use a device tree > and don't use the BSP I'm providing where switching is a no brainer. Yeah, let's do it now before there are users, if that's OK with you. >> What's this binding for anyway? It looks a lot like an ad-hoc pinctrl >> binding for configuring pinout, is that accurate? > Well, it has to do with pinctrl, but it was agreed that managing it with > a single pinctrl device would complicate things considerably and in the > end the device tree wouldn't describe how the hardware looks like but > would result in a lot of complexity just to fit into the Linux pinctrl > model. > > See the thread around > http://marc.info/?l=linux-spi&m=137485291802157&w=2 for the glory > details. OK, thanks for the background. -Olof
diff --git a/arch/arm/boot/dts/efm32gg-dk3750.dts b/arch/arm/boot/dts/efm32gg-dk3750.dts index b4031fa4a567..d5dd2a2a7970 100644 --- a/arch/arm/boot/dts/efm32gg-dk3750.dts +++ b/arch/arm/boot/dts/efm32gg-dk3750.dts @@ -43,7 +43,7 @@ spi0: spi@4000c000 { /* USART0 */ cs-gpios = <&gpio 68 1>; // E4 - location = <1>; + efm32,location = <1>; status = "ok"; microsd@0 { @@ -57,7 +57,7 @@ spi1: spi@4000c400 { /* USART1 */ cs-gpios = <&gpio 51 1>; // D3 - location = <1>; + efm32,location = <1>; status = "ok"; ks8851@0 { @@ -70,7 +70,7 @@ }; uart4: uart@4000e400 { /* UART1 */ - location = <2>; + efm32,location = <2>; status = "ok"; };
Now that both spi and serial driver support these (commits f2bb31057a42 (spi: efm32: properly namespace location property) and 74be65a3cff5 (serial: efm32: properly namespace location property)) use the better names. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- arch/arm/boot/dts/efm32gg-dk3750.dts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)