* [PATCH] mtd: m25p80: Modify the name of mtd_info
@ 2014-03-21 11:16 Hou Zhiqiang
2014-03-21 17:34 ` Scott Wood
2014-04-16 6:48 ` Brian Norris
0 siblings, 2 replies; 4+ messages in thread
From: Hou Zhiqiang @ 2014-03-21 11:16 UTC (permalink / raw)
To: linux-mtd, linuxppc-dev; +Cc: scottwood, mingkai.hu, Hou Zhiqiang, dwmw2
To specify spi flash layouts by "mtdparts=..." in cmdline, we must
give mtd_info a fixed name,because the cmdlinepart's parser will
match the name of mtd_info given in cmdline.
Now, if use DT, the mtd_info's name will be spi->dev->name. It
consists of spi_master->bus_num, and the spi_master->bus_num maybe
dynamically fetched. So, in this case, replace the component bus_num
with thei physical address of spi master.
Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
---
drivers/mtd/devices/m25p80.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 7eda71d..64450a2 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -30,6 +30,7 @@
#include <linux/mtd/cfi.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
+#include <linux/of_address.h>
#include <linux/of_platform.h>
#include <linux/spi/spi.h>
@@ -934,9 +935,11 @@ static int m25p_probe(struct spi_device *spi)
struct flash_platform_data *data;
struct m25p *flash;
struct flash_info *info;
- unsigned i;
+ unsigned i, ret;
struct mtd_part_parser_data ppdata;
struct device_node *np = spi->dev.of_node;
+ struct resource res;
+ struct device_node *mnp = spi->master->dev.of_node;
/* Platform data helps sort out which chip type we have, as
* well as how this board partitions it. If we don't have
@@ -1009,8 +1012,17 @@ static int m25p_probe(struct spi_device *spi)
if (data && data->name)
flash->mtd.name = data->name;
- else
- flash->mtd.name = dev_name(&spi->dev);
+ else{
+ ret = of_address_to_resource(mnp, 0, &res);
+ if (ret) {
+ dev_err(&spi->dev, "failed to get spi master resource\n");
+ return ret;
+ }
+ flash->mtd.name = kasprintf(GFP_KERNEL, "spi%x.%d",
+ (unsigned)res.start, spi->chip_select);
+ if (!flash->mtd.name)
+ return -ENOMEM;
+ }
flash->mtd.type = MTD_NORFLASH;
flash->mtd.writesize = 1;
--
1.8.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: m25p80: Modify the name of mtd_info
2014-03-21 11:16 [PATCH] mtd: m25p80: Modify the name of mtd_info Hou Zhiqiang
@ 2014-03-21 17:34 ` Scott Wood
2014-04-16 6:48 ` Brian Norris
1 sibling, 0 replies; 4+ messages in thread
From: Scott Wood @ 2014-03-21 17:34 UTC (permalink / raw)
To: Hou Zhiqiang; +Cc: linuxppc-dev, mingkai.hu, linux-mtd, dwmw2
On Fri, 2014-03-21 at 19:16 +0800, Hou Zhiqiang wrote:
> @@ -1009,8 +1012,17 @@ static int m25p_probe(struct spi_device *spi)
>
> if (data && data->name)
> flash->mtd.name = data->name;
> - else
> - flash->mtd.name = dev_name(&spi->dev);
> + else{
Whitespace
> + ret = of_address_to_resource(mnp, 0, &res);
> + if (ret) {
> + dev_err(&spi->dev, "failed to get spi master resource\n");
> + return ret;
> + }
> + flash->mtd.name = kasprintf(GFP_KERNEL, "spi%x.%d",
> + (unsigned)res.start, spi->chip_select);
Don't use "unsigned" by itself. Don't cast physical addresses to
"unsigned int" -- use "unsigned long long".
-Scott
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mtd: m25p80: Modify the name of mtd_info
2014-03-21 11:16 [PATCH] mtd: m25p80: Modify the name of mtd_info Hou Zhiqiang
2014-03-21 17:34 ` Scott Wood
@ 2014-04-16 6:48 ` Brian Norris
2014-04-21 10:21 ` B48286
1 sibling, 1 reply; 4+ messages in thread
From: Brian Norris @ 2014-04-16 6:48 UTC (permalink / raw)
To: Hou Zhiqiang; +Cc: scottwood, linuxppc-dev, mingkai.hu, linux-mtd, dwmw2
On Fri, Mar 21, 2014 at 07:16:18PM +0800, Hou Zhiqiang wrote:
> To specify spi flash layouts by "mtdparts=..." in cmdline, we must
> give mtd_info a fixed name,because the cmdlinepart's parser will
> match the name of mtd_info given in cmdline.
> Now, if use DT, the mtd_info's name will be spi->dev->name. It
> consists of spi_master->bus_num, and the spi_master->bus_num maybe
> dynamically fetched. So, in this case, replace the component bus_num
> with thei physical address of spi master.
>
> Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
> ---
> drivers/mtd/devices/m25p80.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 7eda71d..64450a2 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -30,6 +30,7 @@
> #include <linux/mtd/cfi.h>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/partitions.h>
> +#include <linux/of_address.h>
> #include <linux/of_platform.h>
>
> #include <linux/spi/spi.h>
> @@ -934,9 +935,11 @@ static int m25p_probe(struct spi_device *spi)
> struct flash_platform_data *data;
> struct m25p *flash;
> struct flash_info *info;
> - unsigned i;
> + unsigned i, ret;
> struct mtd_part_parser_data ppdata;
> struct device_node *np = spi->dev.of_node;
> + struct resource res;
> + struct device_node *mnp = spi->master->dev.of_node;
>
> /* Platform data helps sort out which chip type we have, as
> * well as how this board partitions it. If we don't have
> @@ -1009,8 +1012,17 @@ static int m25p_probe(struct spi_device *spi)
>
> if (data && data->name)
> flash->mtd.name = data->name;
> - else
> - flash->mtd.name = dev_name(&spi->dev);
> + else{
> + ret = of_address_to_resource(mnp, 0, &res);
You're making a lot assumptions about the SPI master/device
relationship -- that the master has a device-node; that the first
resource of the master is its physical address (this is not guaranteed
by DT semantics). Please do not do this.
> + if (ret) {
> + dev_err(&spi->dev, "failed to get spi master resource\n");
> + return ret;
This is wrong. It breaks all !CONFIG_OF cases which didn't specify a
name via platform data.
> + }
> + flash->mtd.name = kasprintf(GFP_KERNEL, "spi%x.%d",
> + (unsigned)res.start, spi->chip_select);
> + if (!flash->mtd.name)
> + return -ENOMEM;
> + }
>
> flash->mtd.type = MTD_NORFLASH;
> flash->mtd.writesize = 1;
Brian
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] mtd: m25p80: Modify the name of mtd_info
2014-04-16 6:48 ` Brian Norris
@ 2014-04-21 10:21 ` B48286
0 siblings, 0 replies; 4+ messages in thread
From: B48286 @ 2014-04-21 10:21 UTC (permalink / raw)
To: 'Brian Norris'
Cc: Scott Wood, linuxppc-dev, Mingkai.Hu, linux-mtd, dwmw2
> -----Original Message-----
> From: Brian Norris [mailto:computersforpeace@gmail.com]
> Sent: Wednesday, April 16, 2014 2:48 PM
> To: Hou Zhiqiang-B48286
> Cc: linux-mtd@lists.infradead.org; linuxppc-dev@ozlabs.org; Wood Scott-
> B07421; Hu Mingkai-B21284; dwmw2@infradead.org
> Subject: Re: [PATCH] mtd: m25p80: Modify the name of mtd_info
>
> On Fri, Mar 21, 2014 at 07:16:18PM +0800, Hou Zhiqiang wrote:
> > To specify spi flash layouts by "mtdparts=..." in cmdline, we must
> > give mtd_info a fixed name,because the cmdlinepart's parser will match
> > the name of mtd_info given in cmdline.
> > Now, if use DT, the mtd_info's name will be spi->dev->name. It
> > consists of spi_master->bus_num, and the spi_master->bus_num maybe
> > dynamically fetched. So, in this case, replace the component bus_num
> > with thei physical address of spi master.
> >
> > Signed-off-by: Hou Zhiqiang <B48286@freescale.com>
> > ---
> > drivers/mtd/devices/m25p80.c | 18 +++++++++++++++---
> > 1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/devices/m25p80.c
> > b/drivers/mtd/devices/m25p80.c index 7eda71d..64450a2 100644
> > --- a/drivers/mtd/devices/m25p80.c
> > +++ b/drivers/mtd/devices/m25p80.c
> > @@ -30,6 +30,7 @@
> > #include <linux/mtd/cfi.h>
> > #include <linux/mtd/mtd.h>
> > #include <linux/mtd/partitions.h>
> > +#include <linux/of_address.h>
> > #include <linux/of_platform.h>
> >
> > #include <linux/spi/spi.h>
> > @@ -934,9 +935,11 @@ static int m25p_probe(struct spi_device *spi)
> > struct flash_platform_data *data;
> > struct m25p *flash;
> > struct flash_info *info;
> > - unsigned i;
> > + unsigned i, ret;
> > struct mtd_part_parser_data ppdata;
> > struct device_node *np = spi->dev.of_node;
> > + struct resource res;
> > + struct device_node *mnp = spi->master->dev.of_node;
> >
> > /* Platform data helps sort out which chip type we have, as
> > * well as how this board partitions it. If we don't have @@
> > -1009,8 +1012,17 @@ static int m25p_probe(struct spi_device *spi)
> >
> > if (data && data->name)
> > flash->mtd.name = data->name;
> > - else
> > - flash->mtd.name = dev_name(&spi->dev);
> > + else{
> > + ret = of_address_to_resource(mnp, 0, &res);
>
> You're making a lot assumptions about the SPI master/device relationship
> -- that the master has a device-node; that the first resource of the
> master is its physical address (this is not guaranteed by DT semantics).
> Please do not do this.
Thank you for your tips, but I think it is right to get the physical address
of master using the API of_address_to_resource, because of_address_to_resource
just parse address-resource from device tree.
>
> > + if (ret) {
> > + dev_err(&spi->dev, "failed to get spi master
> resource\n");
> > + return ret;
>
> This is wrong. It breaks all !CONFIG_OF cases which didn't specify a name
> via platform data.
>
I will resend a patch to fix it.
> > + }
> > + flash->mtd.name = kasprintf(GFP_KERNEL, "spi%x.%d",
> > + (unsigned)res.start, spi->chip_select);
> > + if (!flash->mtd.name)
> > + return -ENOMEM;
> > + }
> >
> > flash->mtd.type = MTD_NORFLASH;
> > flash->mtd.writesize = 1;
>
> Brian
>
Thanks,
Zhiqiang
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-04-21 10:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-21 11:16 [PATCH] mtd: m25p80: Modify the name of mtd_info Hou Zhiqiang
2014-03-21 17:34 ` Scott Wood
2014-04-16 6:48 ` Brian Norris
2014-04-21 10:21 ` B48286
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).