| Message ID | 1555960084-8462-1-git-send-email-dagmcr@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [1/2] spi: Micrel eth switch: declare missing of table | expand |
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?
> 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
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
From: Daniel Gomez <dagmcr@gmail.com> Date: Mon, 22 Apr 2019 21:08:03 +0200 > 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> The discussion of where these drivers should really live is interesting, but these two changes are valid so I am applying them. Applied.
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.
diff --git a/drivers/net/phy/spi_ks8995.c b/drivers/net/phy/spi_ks8995.c index 92b64e2..7475cef 100644 --- a/drivers/net/phy/spi_ks8995.c +++ b/drivers/net/phy/spi_ks8995.c @@ -159,6 +159,14 @@ static const struct spi_device_id ks8995_id[] = { }; MODULE_DEVICE_TABLE(spi, ks8995_id); +static const struct of_device_id ks8895_spi_of_match[] = { + { .compatible = "micrel,ks8995" }, + { .compatible = "micrel,ksz8864" }, + { .compatible = "micrel,ksz8795" }, + { }, + }; +MODULE_DEVICE_TABLE(of, ks8895_spi_of_match); + static inline u8 get_chip_id(u8 val) { return (val >> ID1_CHIPID_S) & ID1_CHIPID_M; @@ -526,6 +534,7 @@ static int ks8995_remove(struct spi_device *spi) static struct spi_driver ks8995_driver = { .driver = { .name = "spi-ks8995", + .of_match_table = of_match_ptr(ks8895_spi_of_match), }, .probe = ks8995_probe, .remove = ks8995_remove,
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(+)