Re: [PATCH 1/2] spi: Micrel eth switch: declare missing of table

Re: [PATCH 1/2] spi: Micrel eth switch: declare missing of table

[Heiner Kallweit]

On 22.04.2019 21:08, Daniel Gomez wrote:
> Add missing <of_device_id> table for SPI driver relying on SPI
> device match since compatible is in a DT binding or in a DTS.
> 
> Before this patch:
> modinfo drivers/net/phy/spi_ks8995.ko | grep alias
> alias:          spi:ksz8795
> alias:          spi:ksz8864
> alias:          spi:ks8995
> 
> After this patch:
> modinfo drivers/net/phy/spi_ks8995.ko | grep alias
> alias:          spi:ksz8795
> alias:          spi:ksz8864
> alias:          spi:ks8995
> alias:          of:N*T*Cmicrel,ksz8795C*
> alias:          of:N*T*Cmicrel,ksz8795
> alias:          of:N*T*Cmicrel,ksz8864C*
> alias:          of:N*T*Cmicrel,ksz8864
> alias:          of:N*T*Cmicrel,ks8995C*
> alias:          of:N*T*Cmicrel,ks8995
> 
> Reported-by: Javier Martinez Canillas <javier@dowhile0.org>
> Signed-off-by: Daniel Gomez <dagmcr@gmail.com>
> ---
>  drivers/net/phy/spi_ks8995.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
I wonder why this SPI driver is under drivers/net/phy. Just because
the SPI IP is found on an ethernet switch chip? And even then it
would be wrong, I see no link to a PHY at all. I'd say the driver
belongs to drivers/spi. Shouldn't we move it?

Re: [PATCH 1/2] spi: Micrel eth switch: declare missing of table

[Andrew Lunn]

> I wonder why this SPI driver is under drivers/net/phy. Just because
> the SPI IP is found on an ethernet switch chip? And even then it
> would be wrong, I see no link to a PHY at all. I'd say the driver
> belongs to drivers/spi. Shouldn't we move it?

This driver is old. It is probably from before the time of DSA. At
least in the OpenWRT world, switches have been thought of as PHY
devices, since they connect to a MAC. All the OpenWRT switch drivers
are in the phy directory.

There is no good place for this. It is not a PHY, but it is a network
driver so should be somewhere under driver/net. There are no other net
drivers in driver/spi, etc.

Since there is not a good place for it, not moving it is the easiest
thing to do.

      Andrew

Re: [PATCH 1/2] spi: Micrel eth switch: declare missing of table

[Heiner Kallweit]

On 23.04.2019 14:28, Andrew Lunn wrote:
>> I wonder why this SPI driver is under drivers/net/phy. Just because
>> the SPI IP is found on an ethernet switch chip? And even then it
>> would be wrong, I see no link to a PHY at all. I'd say the driver
>> belongs to drivers/spi. Shouldn't we move it?
> 
> This driver is old. It is probably from before the time of DSA. At
> least in the OpenWRT world, switches have been thought of as PHY
> devices, since they connect to a MAC. All the OpenWRT switch drivers
> are in the phy directory.
> 
> There is no good place for this. It is not a PHY, but it is a network
> driver so should be somewhere under driver/net. There are no other net
> drivers in driver/spi, etc.
> 
Except having "switch" in the name this driver is solely a SPI driver
and it uses no network code at all. And it has no dependency on any
network driver. Therefore I wouldn't consider it a network driver.
Else any functionality found on a SoC would need to be under drivers/soc ;)

> Since there is not a good place for it, not moving it is the easiest
> thing to do.
> 
>       Andrew
> 
Heiner

Re: [PATCH 1/2] spi: Micrel eth switch: declare missing of table

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

On Tue, Apr 23, 2019 at 07:33:10PM +0200, Heiner Kallweit wrote:

> Except having "switch" in the name this driver is solely a SPI driver
> and it uses no network code at all. And it has no dependency on any
> network driver. Therefore I wouldn't consider it a network driver.
> Else any functionality found on a SoC would need to be under drivers/soc ;)

Not seen the code here but what Heiner says makes sense to me - note
also that we did at some point add some network framework support for
ethernet switch chips so the device end of the SPI link should be
supportable via standard frameworks now.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]