From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [net-next: PATCH v4 1/7] device property: Introduce fwnode_get_mac_address() Date: Tue, 23 Jan 2018 01:03:39 +0100 Message-ID: References: <1516278704-17141-1-git-send-email-mw@semihalf.com> <1516278704-17141-2-git-send-email-mw@semihalf.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <1516278704-17141-2-git-send-email-mw@semihalf.com> Sender: linux-kernel-owner@vger.kernel.org To: Marcin Wojtas Cc: Linux Kernel Mailing List , linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org, ACPI Devel Maling List , Graeme Gregory , David Miller , Russell King - ARM Linux , Rafael Wysocki , Andrew Lunn , Florian Fainelli , Antoine Tenart , Thomas Petazzoni , Gregory Clement , stefanc@marvell.com, Nadav Haklai , Neta Zur Hershkovits , Ard Biesheuvel , jaz@semihalf.com, Tomasz Nowicki List-Id: linux-acpi@vger.kernel.org On Thu, Jan 18, 2018 at 1:31 PM, Marcin Wojtas wrote: > Until now there were two almost identical functions for > obtaining MAC address - of_get_mac_address() and, more generic, > device_get_mac_address(). However it is not uncommon, > that the network interface is represented as a child > of the actual controller, hence it is not associated > directly to any struct device, required by the latter > routine. > > This commit allows for getting the MAC address for > children nodes in the ACPI world by introducing a new function - > fwnode_get_mac_address(). This commit also changes > device_get_mac_address() routine to be its wrapper, in order > to prevent unnecessary duplication. > > Signed-off-by: Marcin Wojtas > Acked-by: Rafael J. Wysocki > --- > drivers/base/property.c | 28 ++++++++++++++------ > include/linux/property.h | 2 ++ > 2 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/drivers/base/property.c b/drivers/base/property.c > index 851b1b6..f261d1a 100644 > --- a/drivers/base/property.c > +++ b/drivers/base/property.c > @@ -1153,11 +1153,11 @@ int device_get_phy_mode(struct device *dev) > } > EXPORT_SYMBOL_GPL(device_get_phy_mode); > > -static void *device_get_mac_addr(struct device *dev, > +static void *fwnode_get_mac_addr(struct fwnode_handle *fwnode, > const char *name, char *addr, > int alen) > { > - int ret = device_property_read_u8_array(dev, name, addr, alen); > + int ret = fwnode_property_read_u8_array(fwnode, name, addr, alen); > > if (ret == 0 && alen == ETH_ALEN && is_valid_ether_addr(addr)) > return addr; > @@ -1165,8 +1165,8 @@ static void *device_get_mac_addr(struct device *dev, > } > > /** > - * device_get_mac_address - Get the MAC for a given device > - * @dev: Pointer to the device > + * fwnode_get_mac_address - Get the MAC from the firmware node > + * @fwnode: Pointer to the firmware node > * @addr: Address of buffer to store the MAC in > * @alen: Length of the buffer pointed to by addr, should be ETH_ALEN > * > @@ -1187,19 +1187,31 @@ static void *device_get_mac_addr(struct device *dev, > * In this case, the real MAC is in 'local-mac-address', and 'mac-address' > * exists but is all zeros. > */ > -void *device_get_mac_address(struct device *dev, char *addr, int alen) > +void *fwnode_get_mac_address(struct fwnode_handle *fwnode, char *addr, int alen) > { > char *res; > > - res = device_get_mac_addr(dev, "mac-address", addr, alen); > + res = fwnode_get_mac_addr(fwnode, "mac-address", addr, alen); > if (res) > return res; > > - res = device_get_mac_addr(dev, "local-mac-address", addr, alen); > + res = fwnode_get_mac_addr(fwnode, "local-mac-address", addr, alen); > if (res) > return res; > > - return device_get_mac_addr(dev, "address", addr, alen); > + return fwnode_get_mac_addr(fwnode, "address", addr, alen); > +} > +EXPORT_SYMBOL(fwnode_get_mac_address); That should be EXPORT_SYMBOL_GPL(). I have overlooked that previously, sorry about that. > + > +/** > + * device_get_mac_address - Get the MAC for a given device > + * @dev: Pointer to the device > + * @addr: Address of buffer to store the MAC in > + * @alen: Length of the buffer pointed to by addr, should be ETH_ALEN > + */ > +void *device_get_mac_address(struct device *dev, char *addr, int alen) > +{ > + return fwnode_get_mac_address(dev_fwnode(dev), addr, alen); > } > EXPORT_SYMBOL(device_get_mac_address); Same here. Generally speaking, you should use EXPORT_SYMBOL_GPL() everywhere unless there's a specific reason for not doing that in which cases that specific reason has to be clearly spelled out at least in the changelog of the patch, but really better in a code comment. > > diff --git a/include/linux/property.h b/include/linux/property.h > index f6189a3..35620e0 100644 > --- a/include/linux/property.h > +++ b/include/linux/property.h > @@ -279,6 +279,8 @@ int device_get_phy_mode(struct device *dev); > > void *device_get_mac_address(struct device *dev, char *addr, int alen); > > +void *fwnode_get_mac_address(struct fwnode_handle *fwnode, > + char *addr, int alen); > struct fwnode_handle *fwnode_graph_get_next_endpoint( > const struct fwnode_handle *fwnode, struct fwnode_handle *prev); > struct fwnode_handle * > -- From mboxrd@z Thu Jan 1 00:00:00 1970 From: rafael@kernel.org (Rafael J. Wysocki) Date: Tue, 23 Jan 2018 01:03:39 +0100 Subject: [net-next: PATCH v4 1/7] device property: Introduce fwnode_get_mac_address() In-Reply-To: <1516278704-17141-2-git-send-email-mw@semihalf.com> References: <1516278704-17141-1-git-send-email-mw@semihalf.com> <1516278704-17141-2-git-send-email-mw@semihalf.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Jan 18, 2018 at 1:31 PM, Marcin Wojtas wrote: > Until now there were two almost identical functions for > obtaining MAC address - of_get_mac_address() and, more generic, > device_get_mac_address(). However it is not uncommon, > that the network interface is represented as a child > of the actual controller, hence it is not associated > directly to any struct device, required by the latter > routine. > > This commit allows for getting the MAC address for > children nodes in the ACPI world by introducing a new function - > fwnode_get_mac_address(). This commit also changes > device_get_mac_address() routine to be its wrapper, in order > to prevent unnecessary duplication. > > Signed-off-by: Marcin Wojtas > Acked-by: Rafael J. Wysocki > --- > drivers/base/property.c | 28 ++++++++++++++------ > include/linux/property.h | 2 ++ > 2 files changed, 22 insertions(+), 8 deletions(-) > > diff --git a/drivers/base/property.c b/drivers/base/property.c > index 851b1b6..f261d1a 100644 > --- a/drivers/base/property.c > +++ b/drivers/base/property.c > @@ -1153,11 +1153,11 @@ int device_get_phy_mode(struct device *dev) > } > EXPORT_SYMBOL_GPL(device_get_phy_mode); > > -static void *device_get_mac_addr(struct device *dev, > +static void *fwnode_get_mac_addr(struct fwnode_handle *fwnode, > const char *name, char *addr, > int alen) > { > - int ret = device_property_read_u8_array(dev, name, addr, alen); > + int ret = fwnode_property_read_u8_array(fwnode, name, addr, alen); > > if (ret == 0 && alen == ETH_ALEN && is_valid_ether_addr(addr)) > return addr; > @@ -1165,8 +1165,8 @@ static void *device_get_mac_addr(struct device *dev, > } > > /** > - * device_get_mac_address - Get the MAC for a given device > - * @dev: Pointer to the device > + * fwnode_get_mac_address - Get the MAC from the firmware node > + * @fwnode: Pointer to the firmware node > * @addr: Address of buffer to store the MAC in > * @alen: Length of the buffer pointed to by addr, should be ETH_ALEN > * > @@ -1187,19 +1187,31 @@ static void *device_get_mac_addr(struct device *dev, > * In this case, the real MAC is in 'local-mac-address', and 'mac-address' > * exists but is all zeros. > */ > -void *device_get_mac_address(struct device *dev, char *addr, int alen) > +void *fwnode_get_mac_address(struct fwnode_handle *fwnode, char *addr, int alen) > { > char *res; > > - res = device_get_mac_addr(dev, "mac-address", addr, alen); > + res = fwnode_get_mac_addr(fwnode, "mac-address", addr, alen); > if (res) > return res; > > - res = device_get_mac_addr(dev, "local-mac-address", addr, alen); > + res = fwnode_get_mac_addr(fwnode, "local-mac-address", addr, alen); > if (res) > return res; > > - return device_get_mac_addr(dev, "address", addr, alen); > + return fwnode_get_mac_addr(fwnode, "address", addr, alen); > +} > +EXPORT_SYMBOL(fwnode_get_mac_address); That should be EXPORT_SYMBOL_GPL(). I have overlooked that previously, sorry about that. > + > +/** > + * device_get_mac_address - Get the MAC for a given device > + * @dev: Pointer to the device > + * @addr: Address of buffer to store the MAC in > + * @alen: Length of the buffer pointed to by addr, should be ETH_ALEN > + */ > +void *device_get_mac_address(struct device *dev, char *addr, int alen) > +{ > + return fwnode_get_mac_address(dev_fwnode(dev), addr, alen); > } > EXPORT_SYMBOL(device_get_mac_address); Same here. Generally speaking, you should use EXPORT_SYMBOL_GPL() everywhere unless there's a specific reason for not doing that in which cases that specific reason has to be clearly spelled out at least in the changelog of the patch, but really better in a code comment. > > diff --git a/include/linux/property.h b/include/linux/property.h > index f6189a3..35620e0 100644 > --- a/include/linux/property.h > +++ b/include/linux/property.h > @@ -279,6 +279,8 @@ int device_get_phy_mode(struct device *dev); > > void *device_get_mac_address(struct device *dev, char *addr, int alen); > > +void *fwnode_get_mac_address(struct fwnode_handle *fwnode, > + char *addr, int alen); > struct fwnode_handle *fwnode_graph_get_next_endpoint( > const struct fwnode_handle *fwnode, struct fwnode_handle *prev); > struct fwnode_handle * > --