Messages in this thread | | | Date | Thu, 29 Jun 2023 09:59:56 +0800 | Subject | Re: [PATCH net-next 08/10] net: mdio: Remove unnecessary (void*) conversions | From | yunchuan <> |
| |
On 2023/6/28 17:50, Russell King (Oracle) wrote: > Hi, > > I think you missed one case: > > if (mdio_id == XGENE_MDIO_RGMII) { > mdio_bus->read = xgene_mdio_rgmii_read; > mdio_bus->write = xgene_mdio_rgmii_write; > mdio_bus->priv = (void __force *)pdata; > > This cast using __force is also not required. > > On Wed, Jun 28, 2023 at 10:45:17AM +0800, wuych wrote: >> @@ -211,7 +211,7 @@ static void xgene_enet_wr_mdio_csr(void __iomem *base_addr, >> static int xgene_xfi_mdio_write(struct mii_bus *bus, int phy_id, >> int reg, u16 data) >> { >> - void __iomem *addr = (void __iomem *)bus->priv; >> + void __iomem *addr = bus->priv; >> int timeout = 100; >> u32 status, val; >> >> @@ -234,7 +234,7 @@ static int xgene_xfi_mdio_write(struct mii_bus *bus, int phy_id, >> >> static int xgene_xfi_mdio_read(struct mii_bus *bus, int phy_id, int reg) >> { >> - void __iomem *addr = (void __iomem *)bus->priv; >> + void __iomem *addr = bus->priv; >> u32 data, status, val; >> int timeout = 100; > These probably cause Sparse to warn whether or not the cast is there.
Hi, Russell King,
I didn't notice this Sparse warning. Should I remove this cast although it cause Sparse warning? > > Given that in this case, bus->priv is initialised via: > > mdio_bus->priv = (void __force *)pdata->mdio_csr_addr; > > I think the simple thing is to _always_ initialise mdio_bus->priv > to point at pdata, and have xgene_xfi_mdio_*() always do: > > struct xgene_mdio_pdata *pdata = bus->priv; > void __iomem *addr = pdata->mdio_csr_addr; > > The extra access will be dwarfed by the time taken to perform the > access. > > This change should be made with a separate patch and not combined with > the patch removing the casts in xgene_mdio_rgmii_*(). yeah, this change is great. I will send a separate patch as your suggestion If we can ignore Sparse warning. Thanks for your suggestion!
wuych
> > Thanks. >
| |