| Message ID | 20200323234303.526748-4-marex@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: ks8851: Unify KS8851 SPI and MLL drivers | expand |
On Tue, Mar 24, 2020 at 12:42:52AM +0100, Marek Vasut wrote: > Since the driver probe function already has a struct device *dev pointer, > pass it as a parameter to ks8851_init_mac() to avoid fishing it out via > ks->spidev. This is the only reference to spidev in the function, so get > rid of it. This is done in preparation for unifying the KS8851 SPI and > parallel drivers. > > No functional change. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: David S. Miller <davem@davemloft.net> > Cc: Lukas Wunner <lukas@wunner.de> > Cc: Petr Stetiar <ynezz@true.cz> > Cc: YueHaibing <yuehaibing@huawei.com> > --- > drivers/net/ethernet/micrel/ks8851.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c > index 8f4d7c0af723..601a74d750b2 100644 > --- a/drivers/net/ethernet/micrel/ks8851.c > +++ b/drivers/net/ethernet/micrel/ks8851.c > @@ -409,6 +409,7 @@ static void ks8851_read_mac_addr(struct net_device *dev) > /** > * ks8851_init_mac - initialise the mac address > * @ks: The device structure > + * @ddev: The device structure pointer > * > * Get or create the initial mac address for the device and then set that > * into the station address register. A mac address supplied in the device > @@ -416,12 +417,12 @@ static void ks8851_read_mac_addr(struct net_device *dev) > * we try that. If no valid mac address is found we use eth_random_addr() > * to create a new one. > */ > -static void ks8851_init_mac(struct ks8851_net *ks) > +static void ks8851_init_mac(struct ks8851_net *ks, struct device *ddev) > { > struct net_device *dev = ks->netdev; > const u8 *mac_addr; > > - mac_addr = of_get_mac_address(ks->spidev->dev.of_node); > + mac_addr = of_get_mac_address(ddev->of_node); Hi Marek The name ddev is a bit odd. Looking at the code, i see why. dev is normally a struct net_device, which this function already has. You could avoid this oddness by directly passing of_node. Andrew
On Tue, Mar 24, 2020 at 02:06:22AM +0100, Andrew Lunn wrote: > On Tue, Mar 24, 2020 at 12:42:52AM +0100, Marek Vasut wrote: > > Since the driver probe function already has a struct device *dev pointer, > > pass it as a parameter to ks8851_init_mac() to avoid fishing it out via > > ks->spidev. This is the only reference to spidev in the function, so get > > rid of it. This is done in preparation for unifying the KS8851 SPI and > > parallel drivers. [...] > > -static void ks8851_init_mac(struct ks8851_net *ks) > > +static void ks8851_init_mac(struct ks8851_net *ks, struct device *ddev) > > { > > struct net_device *dev = ks->netdev; > > const u8 *mac_addr; > > > > - mac_addr = of_get_mac_address(ks->spidev->dev.of_node); > > + mac_addr = of_get_mac_address(ddev->of_node); > > The name ddev is a bit odd. Looking at the code, i see why. dev is > normally a struct net_device, which this function already has. > > You could avoid this oddness by directly passing of_node. Actually after adding the invocation of of_get_mac_address() with commit 566bd54b067d ("net: ks8851: Support DT-provided MAC address") I've had regrets that I should have used device_get_mac_address() instead since it's platform-agnostic, hence would work with ACPI as well as DT-based systems. device_get_mac_address() needs a struct device, so I'd prefer using that instead of passing an of_node. I agree that "ddev" is somewhat odd. Some drivers name it "device" or "pdev" (which however collides with the naming of platform_devices). Another idea would be to move the handy ndev_to_dev() static inline from apm/xgene/xgene_enet_main.h to include/linux/netdevice.h and use that with "struct net_device *dev", which we already have in ks8851_init_mac(). Thanks, Lukas
diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c index 8f4d7c0af723..601a74d750b2 100644 --- a/drivers/net/ethernet/micrel/ks8851.c +++ b/drivers/net/ethernet/micrel/ks8851.c @@ -409,6 +409,7 @@ static void ks8851_read_mac_addr(struct net_device *dev) /** * ks8851_init_mac - initialise the mac address * @ks: The device structure + * @ddev: The device structure pointer * * Get or create the initial mac address for the device and then set that * into the station address register. A mac address supplied in the device @@ -416,12 +417,12 @@ static void ks8851_read_mac_addr(struct net_device *dev) * we try that. If no valid mac address is found we use eth_random_addr() * to create a new one. */ -static void ks8851_init_mac(struct ks8851_net *ks) +static void ks8851_init_mac(struct ks8851_net *ks, struct device *ddev) { struct net_device *dev = ks->netdev; const u8 *mac_addr; - mac_addr = of_get_mac_address(ks->spidev->dev.of_node); + mac_addr = of_get_mac_address(ddev->of_node); if (!IS_ERR(mac_addr)) { ether_addr_copy(dev->dev_addr, mac_addr); ks8851_write_mac_addr(dev); @@ -1544,7 +1545,7 @@ static int ks8851_probe(struct spi_device *spi) ks->rc_ccr = ks8851_rdreg16(ks, KS_CCR); ks8851_read_selftest(ks); - ks8851_init_mac(ks); + ks8851_init_mac(ks, dev); ret = register_netdev(ndev); if (ret) {
Since the driver probe function already has a struct device *dev pointer, pass it as a parameter to ks8851_init_mac() to avoid fishing it out via ks->spidev. This is the only reference to spidev in the function, so get rid of it. This is done in preparation for unifying the KS8851 SPI and parallel drivers. No functional change. Signed-off-by: Marek Vasut <marex@denx.de> Cc: David S. Miller <davem@davemloft.net> Cc: Lukas Wunner <lukas@wunner.de> Cc: Petr Stetiar <ynezz@true.cz> Cc: YueHaibing <yuehaibing@huawei.com> --- drivers/net/ethernet/micrel/ks8851.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)