Thread (9 messages) 9 messages, 3 authors, 2019-07-24

RE: [PATCH net-next 1/3] enetc: Add mdio bus driver for the PCIe MDIO endpoint

From: Claudiu Manoil <claudiu.manoil@nxp.com>
Date: 2019-07-24 12:58:21
Also in: linux-devicetree, lkml, netdev

-----Original Message-----
From: netdev-owner@vger.kernel.org <redacted> On
Behalf Of Claudiu Manoil
Sent: Wednesday, July 24, 2019 12:53 PM
To: Andrew Lunn <andrew@lunn.ch>
Cc: David S . Miller <davem@davemloft.net>; devicetree@vger.kernel.org;
netdev@vger.kernel.org; Alexandru Marginean
[off-list ref]; linux-kernel@vger.kernel.org; Leo Li
[off-list ref]; Rob Herring [off-list ref]; linux-arm-
kernel@lists.infradead.org
Subject: RE: [PATCH net-next 1/3] enetc: Add mdio bus driver for the PCIe MDIO
endpoint
quoted
-----Original Message-----
From: Andrew Lunn <andrew@lunn.ch>
Sent: Wednesday, July 24, 2019 1:25 AM
To: Claudiu Manoil <claudiu.manoil@nxp.com>
Cc: David S . Miller <davem@davemloft.net>; devicetree@vger.kernel.org;
netdev@vger.kernel.org; Alexandru Marginean
[off-list ref]; linux-kernel@vger.kernel.org; Leo Li
[off-list ref]; Rob Herring [off-list ref]; linux-arm-
kernel@lists.infradead.org
Subject: Re: [PATCH net-next 1/3] enetc: Add mdio bus driver for the
PCIe MDIO endpoint
quoted
+	bus = mdiobus_alloc_size(sizeof(u32 *));
+	if (!bus)
+		return -ENOMEM;
+
quoted
+	bus->priv = pci_iomap_range(pdev, 0, ENETC_MDIO_REG_OFFSET, 0);
This got me confused for a while. You allocate space for a u32 pointer.
bus->priv will point to this space. However, you are not using this
space, you {ab}use the pointer to directly hold the return from
pci_iomap_range(). This works, but sparse is probably unhappy, and you
are wasting the space the u32 pointer takes.
Thanks Andrew,
This is not what I wanted to do, don't ask me how I got to this, it's confusing
indeed.
What's needed here is mdiobus_alloc() or better, devm_mdiobus_alloc().
I've got to do some cleanup in the local mdio bus probing too.
Will send v2.
This is tricky actually, mdiobus_alloc won't do it, I still need to allocate space
for the pointer.
So it's not ok to store the register space pointer directly into priv
(even if iomap returns void *), it's unusual.
Looks like I will have to use double pointers!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help