| Message ID | 20180718161035.7005-6-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 18, 2018 at 06:10:35PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > MTD doesn't support nvmem yet. Some platforms use MTD to read the MAC > address from SPI flash. If we want this function to generalize reading > the MAC address, we need to separately try to use MTD. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > --- > net/ethernet/eth.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c > index adf5bd03851f..f7dbd2cff7f9 100644 > --- a/net/ethernet/eth.c > +++ b/net/ethernet/eth.c > @@ -55,6 +55,7 @@ > #include <linux/of_net.h> > #include <linux/pci.h> > #include <linux/nvmem-consumer.h> > +#include <linux/mtd/mtd.h> > #include <net/dst.h> > #include <net/arp.h> > #include <net/sock.h> > @@ -573,6 +574,25 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr) > } > } > > +#ifdef CONFIG_MTD > + /* NOTE: this should go away as soon as MTD gets nvmem support. */ > + if (!addr) { > + struct mtd_info *mtd; > + int rv; > + > + mtd = get_mtd_device_nm("MAC-Address"); In order for this to go away, you need to keep backwards compatibility. When using nvmem, you look for a cell called "mac-address". Here you are looking for "MAC-Address". That is going to make backwards compatibility harder. How do you plan to do it? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2018-07-18 18:47 GMT+02:00 Andrew Lunn <andrew@lunn.ch>: > On Wed, Jul 18, 2018 at 06:10:35PM +0200, Bartosz Golaszewski wrote: >> From: Bartosz Golaszewski <bgolaszewski@baylibre.com> >> >> MTD doesn't support nvmem yet. Some platforms use MTD to read the MAC >> address from SPI flash. If we want this function to generalize reading >> the MAC address, we need to separately try to use MTD. >> >> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >> --- >> net/ethernet/eth.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c >> index adf5bd03851f..f7dbd2cff7f9 100644 >> --- a/net/ethernet/eth.c >> +++ b/net/ethernet/eth.c >> @@ -55,6 +55,7 @@ >> #include <linux/of_net.h> >> #include <linux/pci.h> >> #include <linux/nvmem-consumer.h> >> +#include <linux/mtd/mtd.h> >> #include <net/dst.h> >> #include <net/arp.h> >> #include <net/sock.h> >> @@ -573,6 +574,25 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr) >> } >> } >> >> +#ifdef CONFIG_MTD >> + /* NOTE: this should go away as soon as MTD gets nvmem support. */ >> + if (!addr) { >> + struct mtd_info *mtd; >> + int rv; >> + >> + mtd = get_mtd_device_nm("MAC-Address"); > > In order for this to go away, you need to keep backwards > compatibility. When using nvmem, you look for a cell called > "mac-address". Here you are looking for "MAC-Address". That is going > to make backwards compatibility harder. How do you plan to do it? > > Andrew I'm trying to adjust to already existing users. The only user of get_mtd_device_nm() who calls it to read the MAC address registers a partition called "MAC-Address". We can't change it since it's visible from user space. In the future we'd just have to have a list of supported string that we'd use to do the nvmem lookup. Bart -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> +#ifdef CONFIG_MTD > >> + /* NOTE: this should go away as soon as MTD gets nvmem support. */ > >> + if (!addr) { > >> + struct mtd_info *mtd; > >> + int rv; > >> + > >> + mtd = get_mtd_device_nm("MAC-Address"); > > > > In order for this to go away, you need to keep backwards > > compatibility. When using nvmem, you look for a cell called > > "mac-address". Here you are looking for "MAC-Address". That is going > > to make backwards compatibility harder. How do you plan to do it? > > > > Andrew > > I'm trying to adjust to already existing users. The only user of > get_mtd_device_nm() who calls it to read the MAC address registers a > partition called "MAC-Address". We can't change it since it's visible > from user space. In the future we'd just have to have a list of > supported string that we'd use to do the nvmem lookup. Why not have the nvmem cell called "MAC-Address"? When you add nvmem support to MTD, i assume you are going to map each MTD partition to an nvmem cell? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2018-07-18 19:03 GMT+02:00 Andrew Lunn <andrew@lunn.ch>: >> >> +#ifdef CONFIG_MTD >> >> + /* NOTE: this should go away as soon as MTD gets nvmem support. */ >> >> + if (!addr) { >> >> + struct mtd_info *mtd; >> >> + int rv; >> >> + >> >> + mtd = get_mtd_device_nm("MAC-Address"); >> > >> > In order for this to go away, you need to keep backwards >> > compatibility. When using nvmem, you look for a cell called >> > "mac-address". Here you are looking for "MAC-Address". That is going >> > to make backwards compatibility harder. How do you plan to do it? >> > >> > Andrew >> >> I'm trying to adjust to already existing users. The only user of >> get_mtd_device_nm() who calls it to read the MAC address registers a >> partition called "MAC-Address". We can't change it since it's visible >> from user space. In the future we'd just have to have a list of >> supported string that we'd use to do the nvmem lookup. > > Why not have the nvmem cell called "MAC-Address"? When you add nvmem > support to MTD, i assume you are going to map each MTD partition to an > nvmem cell? Because all existing users of nvmem use "mac-address" as the name of this cell already. I guess we will need to live with both in this particular function. Bart -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 19, 2018 at 10:14:29AM +0200, Bartosz Golaszewski wrote: > 2018-07-18 19:03 GMT+02:00 Andrew Lunn <andrew@lunn.ch>: > >> >> +#ifdef CONFIG_MTD > >> >> + /* NOTE: this should go away as soon as MTD gets nvmem support. */ > >> >> + if (!addr) { > >> >> + struct mtd_info *mtd; > >> >> + int rv; > >> >> + > >> >> + mtd = get_mtd_device_nm("MAC-Address"); > >> > > >> > In order for this to go away, you need to keep backwards > >> > compatibility. When using nvmem, you look for a cell called > >> > "mac-address". Here you are looking for "MAC-Address". That is going > >> > to make backwards compatibility harder. How do you plan to do it? > >> > > >> > Andrew > >> > >> I'm trying to adjust to already existing users. The only user of > >> get_mtd_device_nm() who calls it to read the MAC address registers a > >> partition called "MAC-Address". We can't change it since it's visible > >> from user space. In the future we'd just have to have a list of > >> supported string that we'd use to do the nvmem lookup. > > > > Why not have the nvmem cell called "MAC-Address"? When you add nvmem > > support to MTD, i assume you are going to map each MTD partition to an > > nvmem cell? > > Because all existing users of nvmem use "mac-address" as the name of > this cell already. I guess we will need to live with both in this > particular function. So i'm not convinced this last patch is making things better. I would prefer if it was dropped for the moment. Wait until MTD via nvmem is actually implemented and there is a concrete concept of how a MAC address would be looked up without having lots of ugly code. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2018-07-19 17:01 GMT+02:00 Andrew Lunn <andrew@lunn.ch>: > On Thu, Jul 19, 2018 at 10:14:29AM +0200, Bartosz Golaszewski wrote: >> 2018-07-18 19:03 GMT+02:00 Andrew Lunn <andrew@lunn.ch>: >> >> >> +#ifdef CONFIG_MTD >> >> >> + /* NOTE: this should go away as soon as MTD gets nvmem support. */ >> >> >> + if (!addr) { >> >> >> + struct mtd_info *mtd; >> >> >> + int rv; >> >> >> + >> >> >> + mtd = get_mtd_device_nm("MAC-Address"); >> >> > >> >> > In order for this to go away, you need to keep backwards >> >> > compatibility. When using nvmem, you look for a cell called >> >> > "mac-address". Here you are looking for "MAC-Address". That is going >> >> > to make backwards compatibility harder. How do you plan to do it? >> >> > >> >> > Andrew >> >> >> >> I'm trying to adjust to already existing users. The only user of >> >> get_mtd_device_nm() who calls it to read the MAC address registers a >> >> partition called "MAC-Address". We can't change it since it's visible >> >> from user space. In the future we'd just have to have a list of >> >> supported string that we'd use to do the nvmem lookup. >> > >> > Why not have the nvmem cell called "MAC-Address"? When you add nvmem >> > support to MTD, i assume you are going to map each MTD partition to an >> > nvmem cell? >> >> Because all existing users of nvmem use "mac-address" as the name of >> this cell already. I guess we will need to live with both in this >> particular function. > > So i'm not convinced this last patch is making things better. I would > prefer if it was dropped for the moment. Wait until MTD via nvmem is > actually implemented and there is a concrete concept of how a MAC > address would be looked up without having lots of ugly code. > > Andrew Unfortunately: this would effectively block me from improving the support for older davinci boards. Having a (subjectively) ugly but generalized way of reading the MAC address from MTD is still better than using the MTD notifier from board files IMO. Bart -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Unfortunately: this would effectively block me from improving the > support for older davinci boards. Is there something blocking you from converting the board to device tree? This is something i did with a lot of the Marvell boards a few years ago. For a while, we had both DT and board setup files. After a couple of cycles, we killed off the setup files. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2018-07-19 17:27 GMT+02:00 Andrew Lunn <andrew@lunn.ch>: >> Unfortunately: this would effectively block me from improving the >> support for older davinci boards. > > Is there something blocking you from converting the board to device > tree? This is something i did with a lot of the Marvell boards a few > years ago. For a while, we had both DT and board setup files. After a > couple of cycles, we killed off the setup files. > > Andrew Actually some board are supported both in DT and board files (da850-evm) right now, but Sekhar wants to keep the support via board files in the kernel so that's a no go. Bart -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 19 July 2018 09:05 PM, Bartosz Golaszewski wrote: > 2018-07-19 17:27 GMT+02:00 Andrew Lunn <andrew@lunn.ch>: >>> Unfortunately: this would effectively block me from improving the >>> support for older davinci boards. >> >> Is there something blocking you from converting the board to device >> tree? This is something i did with a lot of the Marvell boards a few >> years ago. For a while, we had both DT and board setup files. After a >> couple of cycles, we killed off the setup files. >> >> Andrew > > Actually some board are supported both in DT and board files > (da850-evm) right now, but Sekhar wants to keep the support via board > files in the kernel so that's a no go. Its not that I want it that way, but we cannot get rid of board files till DT has equivalent support. The bigger issue is not on DA850, but on the 5 older DaVinci SoCs which do not support device-tree based boot today. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 20, 2018 at 10:47:51AM +0530, Sekhar Nori wrote: > On Thursday 19 July 2018 09:05 PM, Bartosz Golaszewski wrote: > > 2018-07-19 17:27 GMT+02:00 Andrew Lunn <andrew@lunn.ch>: > >>> Unfortunately: this would effectively block me from improving the > >>> support for older davinci boards. > >> > >> Is there something blocking you from converting the board to device > >> tree? This is something i did with a lot of the Marvell boards a few > >> years ago. For a while, we had both DT and board setup files. After a > >> couple of cycles, we killed off the setup files. > >> > >> Andrew > > > > Actually some board are supported both in DT and board files > > (da850-evm) right now, but Sekhar wants to keep the support via board > > files in the kernel so that's a no go. > > Its not that I want it that way, but we cannot get rid of board files > till DT has equivalent support. > > The bigger issue is not on DA850, but on the 5 older DaVinci SoCs which > do not support device-tree based boot today. The nice thing about board files is they keep any ugly code near to where it is needed. The proposal here is to put some 'temporary' code in the net core. And it is assumed at some point somebody will write nvmem over MTD, which can be used to replace this temporary code, but that in itself needs an ugly list of special cases when using board files? I would prefer somebody just did the work to convert these 5 boards to DT, with a clean design of how nvmem over MTD would work. Having converted a number of Marvell boards to DT, i have an idea of the effort required. If most of the drivers already support DT, it can be done quickly. So the big job here is probably nvmem over MTD. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c index adf5bd03851f..f7dbd2cff7f9 100644 --- a/net/ethernet/eth.c +++ b/net/ethernet/eth.c @@ -55,6 +55,7 @@ #include <linux/of_net.h> #include <linux/pci.h> #include <linux/nvmem-consumer.h> +#include <linux/mtd/mtd.h> #include <net/dst.h> #include <net/arp.h> #include <net/sock.h> @@ -573,6 +574,25 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr) } } +#ifdef CONFIG_MTD + /* NOTE: this should go away as soon as MTD gets nvmem support. */ + if (!addr) { + struct mtd_info *mtd; + int rv; + + mtd = get_mtd_device_nm("MAC-Address"); + if (!IS_ERR(mtd)) { + rv = mtd_read(mtd, 0, ETH_ALEN, &alen, addrbuf); + if (rv == 0) { + from = "MTD"; + addr = addrbuf; + } + + put_mtd_device(mtd); + } + } +#endif /* CONFIG_MTD */ + if (!addr || !is_valid_ether_addr(addr)) return -ENODEV;