Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1494758 > unrolled thread
| Started by | Robert Jarzmik <robert.jarzmik@free.fr> |
|---|---|
| First post | 2016-10-03 11:10 +0200 |
| Last post | 2016-10-09 03:40 +0200 |
| Articles | 12 — 5 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Robert Jarzmik <robert.jarzmik@free.fr> - 2016-10-03 11:10 +0200
Re: [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Jeremy Linton <jeremy.linton@arm.com> - 2016-10-03 17:30 +0200
Re: [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Robert Jarzmik <robert.jarzmik@free.fr> - 2016-10-03 18:20 +0200
Re: [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Mark Rutland <mark.rutland@arm.com> - 2016-10-03 17:50 +0200
Re: [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Robert Jarzmik <robert.jarzmik@free.fr> - 2016-10-03 18:20 +0200
Re: [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Mark Rutland <mark.rutland@arm.com> - 2016-10-03 19:00 +0200
Re: [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Robert Jarzmik <robert.jarzmik@free.fr> - 2016-10-03 21:20 +0200
Re: [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Robert Jarzmik <robert.jarzmik@free.fr> - 2016-10-06 08:50 +0200
Re: [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Rob Herring <robh@kernel.org> - 2016-10-09 03:40 +0200
Re: [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-10-03 18:20 +0200
Re: [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Mark Rutland <mark.rutland@arm.com> - 2016-10-03 18:50 +0200
Re: [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms Rob Herring <robh@kernel.org> - 2016-10-09 03:40 +0200
| From | Robert Jarzmik <robert.jarzmik@free.fr> |
|---|---|
| Date | 2016-10-03 11:10 +0200 |
| Subject | [PATCH 3/3] net: smsc911x: add u16 workaround for pxa platforms |
| Message-ID | <so7q2-2Wv-27@gated-at.bofh.it> |
Add a workaround for mainstone, idp and stargate2 boards, for u16 writes which must be aligned on 32 bits addresses. Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> --- Documentation/devicetree/bindings/net/smsc911x.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt index 3fed3c124411..224965b7453c 100644 --- a/Documentation/devicetree/bindings/net/smsc911x.txt +++ b/Documentation/devicetree/bindings/net/smsc911x.txt @@ -13,6 +13,8 @@ Optional properties: - reg-io-width : Specify the size (in bytes) of the IO accesses that should be performed on the device. Valid value for SMSC LAN is 2 or 4. If it's omitted or invalid, the size would be 2. +- reg-u16-align4 : Boolean, put in place the workaround the force all + u16 writes to be 32 bits aligned - smsc,irq-active-high : Indicates the IRQ polarity is active-high - smsc,irq-push-pull : Indicates the IRQ type is push-pull - smsc,force-internal-phy : Forces SMSC LAN controller to use -- 2.1.4
[toc] | [next] | [standalone]
| From | Jeremy Linton <jeremy.linton@arm.com> |
|---|---|
| Date | 2016-10-03 17:30 +0200 |
| Message-ID | <sodlL-6F3-3@gated-at.bofh.it> |
| In reply to | #1494758 |
Hi Robert, On 10/03/2016 04:05 AM, Robert Jarzmik wrote: > Add a workaround for mainstone, idp and stargate2 boards, for u16 writes > which must be aligned on 32 bits addresses. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > --- > Documentation/devicetree/bindings/net/smsc911x.txt | 2 ++ > 1 file changed, 2 insertions(+) I think this might be the wrong doc file. I think you want the smsc-lan91c111.txt file. Thanks, > > diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt > index 3fed3c124411..224965b7453c 100644 > --- a/Documentation/devicetree/bindings/net/smsc911x.txt > +++ b/Documentation/devicetree/bindings/net/smsc911x.txt > @@ -13,6 +13,8 @@ Optional properties: > - reg-io-width : Specify the size (in bytes) of the IO accesses that > should be performed on the device. Valid value for SMSC LAN is > 2 or 4. If it's omitted or invalid, the size would be 2. > +- reg-u16-align4 : Boolean, put in place the workaround the force all > + u16 writes to be 32 bits aligned > - smsc,irq-active-high : Indicates the IRQ polarity is active-high > - smsc,irq-push-pull : Indicates the IRQ type is push-pull > - smsc,force-internal-phy : Forces SMSC LAN controller to use >
[toc] | [prev] | [next] | [standalone]
| From | Robert Jarzmik <robert.jarzmik@free.fr> |
|---|---|
| Date | 2016-10-03 18:20 +0200 |
| Message-ID | <soe8a-7gD-5@gated-at.bofh.it> |
| In reply to | #1494914 |
Jeremy Linton <jeremy.linton@arm.com> writes: > Hi Robert, > > On 10/03/2016 04:05 AM, Robert Jarzmik wrote: >> Add a workaround for mainstone, idp and stargate2 boards, for u16 writes >> which must be aligned on 32 bits addresses. >> >> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> >> --- >> Documentation/devicetree/bindings/net/smsc911x.txt | 2 ++ >> 1 file changed, 2 insertions(+) > > I think this might be the wrong doc file. I think you want the > smsc-lan91c111.txt file. Ah yes, thanks for spoting that. Cheers. -- Robert
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2016-10-03 17:50 +0200 |
| Message-ID | <sodF7-6Lm-9@gated-at.bofh.it> |
| In reply to | #1494758 |
On Mon, Oct 03, 2016 at 11:05:53AM +0200, Robert Jarzmik wrote: > Add a workaround for mainstone, idp and stargate2 boards, for u16 writes > which must be aligned on 32 bits addresses. > > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > --- > Documentation/devicetree/bindings/net/smsc911x.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt > index 3fed3c124411..224965b7453c 100644 > --- a/Documentation/devicetree/bindings/net/smsc911x.txt > +++ b/Documentation/devicetree/bindings/net/smsc911x.txt > @@ -13,6 +13,8 @@ Optional properties: > - reg-io-width : Specify the size (in bytes) of the IO accesses that > should be performed on the device. Valid value for SMSC LAN is > 2 or 4. If it's omitted or invalid, the size would be 2. > +- reg-u16-align4 : Boolean, put in place the workaround the force all > + u16 writes to be 32 bits aligned This property name and description is confusing. How exactly does this differ from having reg-io-width = <4>, which is documented immediately above? Thanks, Mark. > - smsc,irq-active-high : Indicates the IRQ polarity is active-high > - smsc,irq-push-pull : Indicates the IRQ type is push-pull > - smsc,force-internal-phy : Forces SMSC LAN controller to use > -- > 2.1.4 >
[toc] | [prev] | [next] | [standalone]
| From | Robert Jarzmik <robert.jarzmik@free.fr> |
|---|---|
| Date | 2016-10-03 18:20 +0200 |
| Message-ID | <soe8a-7gD-9@gated-at.bofh.it> |
| In reply to | #1494921 |
Mark Rutland <mark.rutland@arm.com> writes: > On Mon, Oct 03, 2016 at 11:05:53AM +0200, Robert Jarzmik wrote: >> Add a workaround for mainstone, idp and stargate2 boards, for u16 writes >> which must be aligned on 32 bits addresses. >> >> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> >> --- >> Documentation/devicetree/bindings/net/smsc911x.txt | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt >> index 3fed3c124411..224965b7453c 100644 >> --- a/Documentation/devicetree/bindings/net/smsc911x.txt >> +++ b/Documentation/devicetree/bindings/net/smsc911x.txt >> @@ -13,6 +13,8 @@ Optional properties: >> - reg-io-width : Specify the size (in bytes) of the IO accesses that >> should be performed on the device. Valid value for SMSC LAN is >> 2 or 4. If it's omitted or invalid, the size would be 2. >> +- reg-u16-align4 : Boolean, put in place the workaround the force all >> + u16 writes to be 32 bits aligned > > This property name and description is confusing. > > How exactly does this differ from having reg-io-width = <4>, which is > documented immediately above? reg-io-width specifies the IO size, ie. how many data lines are physically connected from the system bus to the lan adapter. reg-u16-align4 tells that a specific hardware doesn't support 16 bit writes not being 32 bits aligned, or said differently that a "store" 16 bits wide on an address of the format 4*n + 2 deserves a special handling in the driver, while a store 16 bits wide on an address of the format 4*n can follow the simple casual case. I'm pretty open to any name you might suggest, these 3 hardwares I know of are really crazy, you can see them in patch 1/3, in the _SMC_outw_align4() function ... Cheers. -- Robert
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2016-10-03 19:00 +0200 |
| Message-ID | <soeKR-7tO-3@gated-at.bofh.it> |
| In reply to | #1494956 |
On Mon, Oct 03, 2016 at 06:11:23PM +0200, Robert Jarzmik wrote: > Mark Rutland <mark.rutland@arm.com> writes: > > > On Mon, Oct 03, 2016 at 11:05:53AM +0200, Robert Jarzmik wrote: > >> Add a workaround for mainstone, idp and stargate2 boards, for u16 writes > >> which must be aligned on 32 bits addresses. > >> > >> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr> > >> --- > >> Documentation/devicetree/bindings/net/smsc911x.txt | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt > >> index 3fed3c124411..224965b7453c 100644 > >> --- a/Documentation/devicetree/bindings/net/smsc911x.txt > >> +++ b/Documentation/devicetree/bindings/net/smsc911x.txt > >> @@ -13,6 +13,8 @@ Optional properties: > >> - reg-io-width : Specify the size (in bytes) of the IO accesses that > >> should be performed on the device. Valid value for SMSC LAN is > >> 2 or 4. If it's omitted or invalid, the size would be 2. > >> +- reg-u16-align4 : Boolean, put in place the workaround the force all > >> + u16 writes to be 32 bits aligned > > > > This property name and description is confusing. > > > > How exactly does this differ from having reg-io-width = <4>, which is > > documented immediately above? > > reg-io-width specifies the IO size, ie. how many data lines are physically > connected from the system bus to the lan adapter. > > reg-u16-align4 tells that a specific hardware doesn't support 16 bit writes not > being 32 bits aligned, or said differently that a "store" 16 bits wide on an > address of the format 4*n + 2 deserves a special handling in the driver, while a > store 16 bits wide on an address of the format 4*n can follow the simple casual > case. If I've understood correctly, effectively the low 2 address lines to the device are hard-wired to zero, e.g. a 16-bit access to 4*n + 2 would go to 4*n + 0 on the device? Or is the failure case distinct from that? Do we have other platforms where similar is true? e.g. u8 accesses requiring 16-bit alignment? Thanks, Mark.
[toc] | [prev] | [next] | [standalone]
| From | Robert Jarzmik <robert.jarzmik@free.fr> |
|---|---|
| Date | 2016-10-03 21:20 +0200 |
| Message-ID | <sogWl-uQ-17@gated-at.bofh.it> |
| In reply to | #1494968 |
Mark Rutland <mark.rutland@arm.com> writes: > On Mon, Oct 03, 2016 at 06:11:23PM +0200, Robert Jarzmik wrote: >> Mark Rutland <mark.rutland@arm.com> writes: >> >> reg-u16-align4 tells that a specific hardware doesn't support 16 bit writes not >> being 32 bits aligned, or said differently that a "store" 16 bits wide on an >> address of the format 4*n + 2 deserves a special handling in the driver, while a >> store 16 bits wide on an address of the format 4*n can follow the simple casual >> case. > > If I've understood correctly, effectively the low 2 address lines to the > device are hard-wired to zero, e.g. a 16-bit access to 4*n + 2 would go > to 4*n + 0 on the device? Or is the failure case distinct from that? It is distinct. The "awful truth" is that an FPGA lies between the system bus and the smc91c111. And this FPGA cannot handle correctly the 4*n + 2 u16 writes. > Do we have other platforms where similar is true? e.g. u8 accesses > requiring 16-bit alignment? Not really, ie. not with a alignement requirement. But there are of course these ones are handled by reg-io-width and the SMC_USE_xxx_BITS flags as far as I understand it. These cases are when a platform declares SMC91X_USE_16BIT or SMC91X_USE_32BIT, but not SMC91X_USE_8BIT, which would make me think of : - CONFIG_SH_SH4202_MICRODEV, - CONFIG_M32R - several omap1 boards - 1 sa1100 board - several MMP and realview boards With all these platforms, each u8 access is replaced with a u16 access and a mask / shift + mask. Cheers. -- Robert
[toc] | [prev] | [next] | [standalone]
| From | Robert Jarzmik <robert.jarzmik@free.fr> |
|---|---|
| Date | 2016-10-06 08:50 +0200 |
| Message-ID | <spaFb-4Sw-3@gated-at.bofh.it> |
| In reply to | #1495004 |
Robert Jarzmik <robert.jarzmik@free.fr> writes: > Mark Rutland <mark.rutland@arm.com> writes: > >> On Mon, Oct 03, 2016 at 06:11:23PM +0200, Robert Jarzmik wrote: >>> Mark Rutland <mark.rutland@arm.com> writes: >>> >>> reg-u16-align4 tells that a specific hardware doesn't support 16 bit writes not >>> being 32 bits aligned, or said differently that a "store" 16 bits wide on an >>> address of the format 4*n + 2 deserves a special handling in the driver, while a >>> store 16 bits wide on an address of the format 4*n can follow the simple casual >>> case. >> >> If I've understood correctly, effectively the low 2 address lines to the >> device are hard-wired to zero, e.g. a 16-bit access to 4*n + 2 would go >> to 4*n + 0 on the device? Or is the failure case distinct from that? > It is distinct. > > The "awful truth" is that an FPGA lies between the system bus and the > smc91c111. And this FPGA cannot handle correctly the 4*n + 2 u16 writes. > >> Do we have other platforms where similar is true? e.g. u8 accesses >> requiring 16-bit alignment? > > Not really, ie. not with a alignement requirement. > > But there are of course these ones are handled by reg-io-width and the > SMC_USE_xxx_BITS flags as far as I understand it. These cases are when a > platform declares SMC91X_USE_16BIT or SMC91X_USE_32BIT, but not SMC91X_USE_8BIT, > which would make me think of : > - CONFIG_SH_SH4202_MICRODEV, > - CONFIG_M32R > - several omap1 boards > - 1 sa1100 board > - several MMP and realview boards > > With all these platforms, each u8 access is replaced with a u16 access and a > mask / shift + mask. Or so what should I call this entry if reg-u16-align4 is not a good candidate ? Cheers. -- Robert
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2016-10-09 03:40 +0200 |
| Message-ID | <sqbfQ-6a9-7@gated-at.bofh.it> |
| In reply to | #1496180 |
On Thu, Oct 06, 2016 at 08:47:13AM +0200, Robert Jarzmik wrote: > Robert Jarzmik <robert.jarzmik@free.fr> writes: > > > Mark Rutland <mark.rutland@arm.com> writes: > > > >> On Mon, Oct 03, 2016 at 06:11:23PM +0200, Robert Jarzmik wrote: > >>> Mark Rutland <mark.rutland@arm.com> writes: > >>> > >>> reg-u16-align4 tells that a specific hardware doesn't support 16 bit writes not > >>> being 32 bits aligned, or said differently that a "store" 16 bits wide on an > >>> address of the format 4*n + 2 deserves a special handling in the driver, while a > >>> store 16 bits wide on an address of the format 4*n can follow the simple casual > >>> case. > >> > >> If I've understood correctly, effectively the low 2 address lines to the > >> device are hard-wired to zero, e.g. a 16-bit access to 4*n + 2 would go > >> to 4*n + 0 on the device? Or is the failure case distinct from that? > > It is distinct. > > > > The "awful truth" is that an FPGA lies between the system bus and the > > smc91c111. And this FPGA cannot handle correctly the 4*n + 2 u16 writes. > > > >> Do we have other platforms where similar is true? e.g. u8 accesses > >> requiring 16-bit alignment? > > > > Not really, ie. not with a alignement requirement. > > > > But there are of course these ones are handled by reg-io-width and the > > SMC_USE_xxx_BITS flags as far as I understand it. These cases are when a > > platform declares SMC91X_USE_16BIT or SMC91X_USE_32BIT, but not SMC91X_USE_8BIT, > > which would make me think of : > > - CONFIG_SH_SH4202_MICRODEV, > > - CONFIG_M32R > > - several omap1 boards > > - 1 sa1100 board > > - several MMP and realview boards > > > > With all these platforms, each u8 access is replaced with a u16 access and a > > mask / shift + mask. > > Or so what should I call this entry if reg-u16-align4 is not a good candidate ? This seems broken in a very platform specific way, so perhaps something named based on the platform. Rob
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@armlinux.org.uk> |
|---|---|
| Date | 2016-10-03 18:20 +0200 |
| Message-ID | <soe8a-7gD-11@gated-at.bofh.it> |
| In reply to | #1494921 |
On Mon, Oct 03, 2016 at 04:46:25PM +0100, Mark Rutland wrote:
> On Mon, Oct 03, 2016 at 11:05:53AM +0200, Robert Jarzmik wrote:
> > Add a workaround for mainstone, idp and stargate2 boards, for u16 writes
> > which must be aligned on 32 bits addresses.
> >
> > Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> > ---
> > Documentation/devicetree/bindings/net/smsc911x.txt | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/smsc911x.txt b/Documentation/devicetree/bindings/net/smsc911x.txt
> > index 3fed3c124411..224965b7453c 100644
> > --- a/Documentation/devicetree/bindings/net/smsc911x.txt
> > +++ b/Documentation/devicetree/bindings/net/smsc911x.txt
> > @@ -13,6 +13,8 @@ Optional properties:
> > - reg-io-width : Specify the size (in bytes) of the IO accesses that
> > should be performed on the device. Valid value for SMSC LAN is
> > 2 or 4. If it's omitted or invalid, the size would be 2.
> > +- reg-u16-align4 : Boolean, put in place the workaround the force all
> > + u16 writes to be 32 bits aligned
>
> This property name and description is confusing.
>
> How exactly does this differ from having reg-io-width = <4>, which is
> documented immediately above?
Please note that the binding doc for smsc,lan91c111.txt is slightly wrong
on two counts:
1) compatible property:
compatible = "smsc,lan91c111";
vs the code:
static const struct of_device_id smc91x_match[] = {
{ .compatible = "smsc,lan91c94", },
{ .compatible = "smsc,lan91c111", },
{},
};
MODULE_DEVICE_TABLE(of, smc91x_match);
So the binding document needs to mention that smsc,lan91c94 is a valid
compatible for this device.
2) reg-io-width property:
- reg-io-width : Mask of sizes (in bytes) of the IO accesses that
are supported on the device. Valid value for SMSC LAN91c111 are
1, 2 or 4. If it's omitted or invalid, the size would be 2 meaning
16-bit access only.
The SMC requires at least one of byte or 16-bit access sizes, with
32-bit access sizes being optional on top. So, the legal values here
are: 1, 2, 3, 5, 6, and 7. 4 is illegal, and has never been supported
by the driver.
Note that the driver will always use byte accesses if '1' is specified
and emulate 16-bit accesses. If '2' is specified, the driver will
always use 16-bit accesses, and emulate byte accesses for the 8-bit
registers using a read-modify-write scheme. If '3' is specified, the
driver will use both 16-bit and byte accesses as appropriate for the
register being accessed with no emulation. Byte or 16-bit access are
required for non-data register access.
Including 32-bit accesses on top of this allows the packet transfer
(iow, data register accesses) to use 32-bit access instructions, which
is a performance boost.
Moreover, look at the property name vs the binding description. It's
property name says it's a width, but the description says it's a mask
of sizes - these really aren't the same thing. Once you start
specifying these other legal masks, it makes a nonsense of the "width"
part of the name. It's too late to try and fix this now though.
The binding document really needs to get fixed - I'll try to cook up a
patch during this week to correct these points, but it probably needs
coordination if others are going to be changing this as well.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2016-10-03 18:50 +0200 |
| Message-ID | <soeBb-7qE-9@gated-at.bofh.it> |
| In reply to | #1494959 |
On Mon, Oct 03, 2016 at 05:09:13PM +0100, Russell King - ARM Linux wrote:
> Please note that the binding doc for smsc,lan91c111.txt is slightly wrong
> on two counts:
>
> 1) compatible property:
>
> compatible = "smsc,lan91c111";
>
> vs the code:
>
> static const struct of_device_id smc91x_match[] = {
> { .compatible = "smsc,lan91c94", },
> { .compatible = "smsc,lan91c111", },
> {},
> };
> MODULE_DEVICE_TABLE(of, smc91x_match);
>
> So the binding document needs to mention that smsc,lan91c94 is a valid
> compatible for this device.
Yes, it should.
> 2) reg-io-width property:
>
> - reg-io-width : Mask of sizes (in bytes) of the IO accesses that
> are supported on the device. Valid value for SMSC LAN91c111 are
> 1, 2 or 4. If it's omitted or invalid, the size would be 2 meaning
> 16-bit access only.
> Moreover, look at the property name vs the binding description. It's
> property name says it's a width, but the description says it's a mask
> of sizes - these really aren't the same thing. Once you start
> specifying these other legal masks, it makes a nonsense of the "width"
> part of the name. It's too late to try and fix this now though.
Indeed, as-is this is nonsense. :(
The best we can do here is to add a big fat notice regarding the
misnaming; adding a new property is only giong to cause more confusion.
> The binding document really needs to get fixed - I'll try to cook up a
> patch during this week to correct these points, but it probably needs
> coordination if others are going to be changing this as well.
Thanks for handling both of these.
Given the historical rate of change of the binding document, I suspect
the stuff for pxa platforms is going to be the only potential conflict.
Either all of that can go via the DT tree (independent of any new code),
or we can ack the whole lot and it can all go via the net tree in one
go.
Thanks,
Mark.
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2016-10-09 03:40 +0200 |
| Message-ID | <sqbfQ-6a9-9@gated-at.bofh.it> |
| In reply to | #1494965 |
On Mon, Oct 03, 2016 at 05:42:29PM +0100, Mark Rutland wrote:
> On Mon, Oct 03, 2016 at 05:09:13PM +0100, Russell King - ARM Linux wrote:
> > Please note that the binding doc for smsc,lan91c111.txt is slightly wrong
> > on two counts:
> >
> > 1) compatible property:
> >
> > compatible = "smsc,lan91c111";
> >
> > vs the code:
> >
> > static const struct of_device_id smc91x_match[] = {
> > { .compatible = "smsc,lan91c94", },
> > { .compatible = "smsc,lan91c111", },
> > {},
> > };
> > MODULE_DEVICE_TABLE(of, smc91x_match);
> >
> > So the binding document needs to mention that smsc,lan91c94 is a valid
> > compatible for this device.
>
> Yes, it should.
>
> > 2) reg-io-width property:
> >
> > - reg-io-width : Mask of sizes (in bytes) of the IO accesses that
> > are supported on the device. Valid value for SMSC LAN91c111 are
> > 1, 2 or 4. If it's omitted or invalid, the size would be 2 meaning
> > 16-bit access only.
>
> > Moreover, look at the property name vs the binding description. It's
> > property name says it's a width, but the description says it's a mask
> > of sizes - these really aren't the same thing. Once you start
> > specifying these other legal masks, it makes a nonsense of the "width"
> > part of the name. It's too late to try and fix this now though.
>
> Indeed, as-is this is nonsense. :(
>
> The best we can do here is to add a big fat notice regarding the
> misnaming; adding a new property is only giong to cause more confusion.
Just fix the text here removing the mask part. This is a common property
and not a mask. The rest saying 1, 2, 4 being valid is correct. There
are no occurences using this as a mask in kernel dts files either.
>
> > The binding document really needs to get fixed - I'll try to cook up a
> > patch during this week to correct these points, but it probably needs
> > coordination if others are going to be changing this as well.
>
> Thanks for handling both of these.
>
> Given the historical rate of change of the binding document, I suspect
> the stuff for pxa platforms is going to be the only potential conflict.
>
> Either all of that can go via the DT tree (independent of any new code),
> or we can ack the whole lot and it can all go via the net tree in one
> go.
>
> Thanks,
> Mark.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web