Re: [PATCH 01/27] mtd: nand: introduce function to fix a common bug in most nand-drivers not showing a device in sysfs
On Wed, Nov 5, 2014 at 10:34 AM, Brian Norris wrote: > On Sun, Nov 02, 2014 at 10:03:53PM +0100, Frans Klaver wrote: >> On Wed, May 28, 2014 at 01:43:44AM -0700, Brian Norris wrote: >> > And in fact, if any drivers are missing mtd->name, perhaps it's best to >> > just modify the MTD registration to give them a default: >> > >> > if (!mtd->name) >> > mtd->name = dev_name(&pdev->dev); >> > >> >> ... >> >> > How about we rethink the "helper" approach, and instead just do >> > validation in the core code? This would cover most of the important >> > parts of your helper, I think: >> > >> > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c >> > index d201feeb3ca6..39ba5812a5a3 100644 >> > --- a/drivers/mtd/mtdcore.c >> > +++ b/drivers/mtd/mtdcore.c >> > @@ -397,6 +397,11 @@ int add_mtd_device(struct mtd_info *mtd) >> > if (device_register(&mtd->dev) != 0) >> > goto fail_added; >> > >> > + if (mtd->dev.parent) >> > + mtd->owner = mtd->dev.parent->driver->owner; >> > + else >> > + WARN_ON(1); >> > + >> >> So I've picked this up now. I do largely agree with the suggested >> approach where the validation and default settings are done in the core >> code. There is a problem with this, though. There are MTD devices that >> call mtd_device_parse_register() in the _init() function (such as the >> maps drivers). These don't have a device ready to be used as parent, and >> they would always be throwing this warning. > > Yeah, I came across the same thing. I think gluebi is another example. > >> So either not having a parent device is bad, or it isn't. The comment >> suggests it is, the existing code suggests it isn't. So we'll need to >> make a decision about who's right. > > I think not having a parent is not really bad. It's helpful for tracking > the device hierarchy in sysfs, but it's not strictly necessary. So we > should probably not do anything drastic like WARN_ON() yet. OK. I'll probably add some sane defaults to add_mtd_device() and have the mtd drivers make use of that fact. Since this isn't really critical, I guess fixing the sysfs entry won't need back-porting. >> > if (MTD_DEVT(i)) >> > device_create(&mtd_class, mtd->dev.parent, >> > MTD_DEVT(i) + 1, >> > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c >> > index 1ca9aec141ff..9869bbef50cf 100644 >> > --- a/drivers/mtd/mtdpart.c >> > +++ b/drivers/mtd/mtdpart.c >> > @@ -370,7 +370,6 @@ static struct mtd_part *allocate_partition(struct >> > mtd_info *master, >> > slave->mtd.subpage_sft = master->subpage_sft; >> > >> > slave->mtd.name = name; >> > - slave->mtd.owner = master->owner; >> >> What would be the purpose of removing this line? Owner is already set? >> Can we rely on that? > > I'm not completely sure why I wrote that, but I think the only call site > for alloc_partition() is in mtd_add_partition(), which calls > add_mtd_device(). Alright, I'll have a brief look into that again, then. Thanks, Frans -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/27] mtd: nand: introduce function to fix a common bug in most nand-drivers not showing a device in sysfs
On Sun, Nov 02, 2014 at 10:03:53PM +0100, Frans Klaver wrote: > On Wed, May 28, 2014 at 01:43:44AM -0700, Brian Norris wrote: > > And in fact, if any drivers are missing mtd->name, perhaps it's best to > > just modify the MTD registration to give them a default: > > > > if (!mtd->name) > > mtd->name = dev_name(&pdev->dev); > > > > ... > > > How about we rethink the "helper" approach, and instead just do > > validation in the core code? This would cover most of the important > > parts of your helper, I think: > > > > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c > > index d201feeb3ca6..39ba5812a5a3 100644 > > --- a/drivers/mtd/mtdcore.c > > +++ b/drivers/mtd/mtdcore.c > > @@ -397,6 +397,11 @@ int add_mtd_device(struct mtd_info *mtd) > > if (device_register(&mtd->dev) != 0) > > goto fail_added; > > > > + if (mtd->dev.parent) > > + mtd->owner = mtd->dev.parent->driver->owner; > > + else > > + WARN_ON(1); > > + > > So I've picked this up now. I do largely agree with the suggested > approach where the validation and default settings are done in the core > code. There is a problem with this, though. There are MTD devices that > call mtd_device_parse_register() in the _init() function (such as the > maps drivers). These don't have a device ready to be used as parent, and > they would always be throwing this warning. Yeah, I came across the same thing. I think gluebi is another example. > So either not having a parent device is bad, or it isn't. The comment > suggests it is, the existing code suggests it isn't. So we'll need to > make a decision about who's right. I think not having a parent is not really bad. It's helpful for tracking the device hierarchy in sysfs, but it's not strictly necessary. So we should probably not do anything drastic like WARN_ON() yet. > > if (MTD_DEVT(i)) > > device_create(&mtd_class, mtd->dev.parent, > > MTD_DEVT(i) + 1, > > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c > > index 1ca9aec141ff..9869bbef50cf 100644 > > --- a/drivers/mtd/mtdpart.c > > +++ b/drivers/mtd/mtdpart.c > > @@ -370,7 +370,6 @@ static struct mtd_part *allocate_partition(struct > > mtd_info *master, > > slave->mtd.subpage_sft = master->subpage_sft; > > > > slave->mtd.name = name; > > - slave->mtd.owner = master->owner; > > What would be the purpose of removing this line? Owner is already set? > Can we rely on that? I'm not completely sure why I wrote that, but I think the only call site for alloc_partition() is in mtd_add_partition(), which calls add_mtd_device(). Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/27] mtd: nand: introduce function to fix a common bug in most nand-drivers not showing a device in sysfs
On Wed, May 28, 2014 at 01:43:44AM -0700, Brian Norris wrote: > And in fact, if any drivers are missing mtd->name, perhaps it's best to > just modify the MTD registration to give them a default: > > if (!mtd->name) > mtd->name = dev_name(&pdev->dev); > ... > How about we rethink the "helper" approach, and instead just do > validation in the core code? This would cover most of the important > parts of your helper, I think: > > diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c > index d201feeb3ca6..39ba5812a5a3 100644 > --- a/drivers/mtd/mtdcore.c > +++ b/drivers/mtd/mtdcore.c > @@ -397,6 +397,11 @@ int add_mtd_device(struct mtd_info *mtd) > if (device_register(&mtd->dev) != 0) > goto fail_added; > > + if (mtd->dev.parent) > + mtd->owner = mtd->dev.parent->driver->owner; > + else > + WARN_ON(1); > + So I've picked this up now. I do largely agree with the suggested approach where the validation and default settings are done in the core code. There is a problem with this, though. There are MTD devices that call mtd_device_parse_register() in the _init() function (such as the maps drivers). These don't have a device ready to be used as parent, and they would always be throwing this warning. So either not having a parent device is bad, or it isn't. The comment suggests it is, the existing code suggests it isn't. So we'll need to make a decision about who's right. > if (MTD_DEVT(i)) > device_create(&mtd_class, mtd->dev.parent, > MTD_DEVT(i) + 1, > diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c > index 1ca9aec141ff..9869bbef50cf 100644 > --- a/drivers/mtd/mtdpart.c > +++ b/drivers/mtd/mtdpart.c > @@ -370,7 +370,6 @@ static struct mtd_part *allocate_partition(struct > mtd_info *master, > slave->mtd.subpage_sft = master->subpage_sft; > > slave->mtd.name = name; > - slave->mtd.owner = master->owner; What would be the purpose of removing this line? Owner is already set? Can we rely on that? Thanks, Frans -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/27] mtd: nand: introduce function to fix a common bug in most nand-drivers not showing a device in sysfs
Am 29.05.2014 08:17, schrieb Artem Bityutskiy: On Wed, 2014-05-28 at 23:09 +0200, Alexander Holler wrote: I'm very sorry, but I find such discussions extremly tiresome. Why discussing then at all, just go ahead and to something else. Agreed. In order to maintain psychical health it's better to not get in contact with this kindergarten. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/27] mtd: nand: introduce function to fix a common bug in most nand-drivers not showing a device in sysfs
On Wed, 2014-05-28 at 23:09 +0200, Alexander Holler wrote: > I'm very sorry, but I find such discussions extremly tiresome. Why discussing then at all, just go ahead and to something else. -- Best Regards, Artem Bityutskiy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/27] mtd: nand: introduce function to fix a common bug in most nand-drivers not showing a device in sysfs
Am 28.05.2014 23:49, schrieb Brian Norris: On Wed, May 28, 2014 at 11:09:05PM +0200, Alexander Holler wrote: I'm very sorry, but I find such discussions extremly tiresome. If you just would have suggested that one if to prevent that someone who doesn't c&p existing code would end up with a clobbered name (which he obviously can't miss if he ever would test his new or changed driver), than I maybe would have send a v2 with that if. But all the other noise just made me to want I never had send (again) a patch to LKML. It is almost impossible to avoid such answers and it turns every patch posting into a endless story where people do want to discuss every line or even character of totally silly things. It would help if you brought a more open attitude to the table. Your doesn't accept that other people do write code different than you and you want to tell me I have to bring a more open attitude to the table? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/27] mtd: nand: introduce function to fix a common bug in most nand-drivers not showing a device in sysfs
On Wed, May 28, 2014 at 11:09:05PM +0200, Alexander Holler wrote: > I'm very sorry, but I find such discussions extremly tiresome. > > If you just would have suggested that one if to prevent that someone who > doesn't c&p existing code would end up with a clobbered name (which he > obviously can't miss if he ever would test his new or changed driver), > than I maybe would have send a v2 with that if. But all the other noise > just made me to want I never had send (again) a patch to LKML. It is > almost impossible to avoid such answers and it turns every patch posting > into a endless story where people do want to discuss every line or even > character of totally silly things. It would help if you brought a more open attitude to the table. https://lkml.org/lkml/2014/5/14/574 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/27] mtd: nand: introduce function to fix a common bug in most nand-drivers not showing a device in sysfs
Am 28.05.2014 22:10, schrieb Brian Norris:
> On Wed, May 28, 2014 at 08:52:06PM +0200, Alexander Holler wrote:
>> Am 28.05.2014 10:43, schrieb Brian Norris:
>>> On Tue, May 27, 2014 at 12:12:26AM +0200, Alexander Holler wrote:
+{
+ mtd->priv = priv;
>>>
>>> I don't think you should hide this one here. It will be quite obvious if
>>> a driver didn't stash its private data but tries to access it later. Are
>>> there any drivers that missed this?
>>
>> No, it just saves a line of source in all drivers and I think it
>> fits there. I don't understand why do you think it is hidden.
>
> The function name doesn't make it obvious what it's doing. So all code
> readers will have to follow the definition to see what it's doing. But
> this point is not that important, so I won't argue.
So you think every function does hide important things? How's about C?
It hides which registers are used. Maybe we should go back to assembler.
But I prefer to just press ctrl-} in vim.
+ mtd->owner = pdev->dev.driver->owner;
+ mtd->dev.parent = &pdev->dev;
+ mtd->name = pdev->dev.driver->name;
>>>
>>> I think this is a little dangerous. You're potentially clobbering the
>>> name that a driver already chose here. And why did you pick to use the
It's a dangerous world. And ordering sometimes matters. Feel free to
post another patch where you add an if() or an if (). I don't care which
style you prefer, but checkpatch would want the second one.
And as I currently don't see your argument to use the driver instead of
the platform device, here's an argument why I prefer to use pdev:
someone might want to use the upper layer (here the platform device) in
that silly function in the future. A very personal decision I did., but...
>>> driver name? This gives non-unique names if there is more than one
>>> device instantiated for a driver. That's why some drivers already use
>>> the device name, not the driver name:
>>>
>>> mtd->name = dev_name(&pev->dev);
>>>
>>> And in fact, if any drivers are missing mtd->name, perhaps it's best to
>>> just modify the MTD registration to give them a default:
>>>
>>> if (!mtd->name)
>>> mtd->name = dev_name(&pdev->dev);
>>>
+}
>>
>> I don't clobber any name. The name is set as default before drivers
>> might do this.
BTW. I don't like what dev_name produces. E.g. on one box I use here
devname would produce f400.nand instead orion_nand. So I would have
to name partitons with f400.nand. I don't understand why someone
would prefer f400.nand instead of orion_nand, but I accept that
other people do so. I don't have to understand everything.
> At the moment this is true, but the ordering is somewhat subtle if you
> don't examine the details of mtd_setup_common_members() to see that it
> is assigning a name. I can easily imagine some new driver in which
> someone does:
>
> mtd->name = ...;
> [...]
> mtd_setup_common_members(mtd, priv, pdev);
>
> And doesn't notice that the ordering matters.
>
> You could make this simpler by either
> (1) making mtd_setup_common_members() check for mtd->name==NULL first
> (2) just move the (default) name assignment to the common MTD
> registration, like I suggested
>
+
extern int mtd_device_parse_register(struct mtd_info *mtd,
const char * const *part_probe_types,
struct mtd_part_parser_data *parser_data,
>>>
>>> How about we rethink the "helper" approach, and instead just do
>>> validation in the core code? This would cover most of the important
>>> parts of your helper, I think:
>>
>> Feel free to change all drivers. I like my approach with fixing 21
>> bugs by reducing code by 44 lines.
>
> I appreciate the bug fixes. I am just questioning the approach. Reduced
> (source) code doesn't help anyone if it makes the code less
> maintainable. My suggestions can probably make this more maintainable,
> fix the same bugs, and reduce the source by a similar (albeit smaller)
> number of lines.
>
>> And it offers a common function where future similiarities can be
>> put into too. Of course you can just add 21 lines, but that is not
>> how I do such stuff.
>
> I don't see how your common location helps much more than putting it in
> mtdcore.c like I suggested, without the extra function.
>
>> And I did the patches. If you don't like them, feel free to ignore
>> them. I'm not playing remote keyboard but I do patches like I would
>> do them, not like some arbitrary maintainer would do them. Sorry for
>> the harsh words.
>
> Arbitrary maintainer, eh? I'm simply suggesting that this could be
> accomplished in a better way. If you don't want to take part in the
> review process, then I have no obligation to engage either.
TIMTOWTDI. Humans are different, they think different, they write
different code, they see code different and they prefer different
styles. And what you think is a better way, isn't one
Re: [PATCH 01/27] mtd: nand: introduce function to fix a common bug in most nand-drivers not showing a device in sysfs
On Wed, May 28, 2014 at 08:52:06PM +0200, Alexander Holler wrote:
> Am 28.05.2014 10:43, schrieb Brian Norris:
> >On Tue, May 27, 2014 at 12:12:26AM +0200, Alexander Holler wrote:
> >>+{
> >>+ mtd->priv = priv;
> >
> >I don't think you should hide this one here. It will be quite obvious if
> >a driver didn't stash its private data but tries to access it later. Are
> >there any drivers that missed this?
>
> No, it just saves a line of source in all drivers and I think it
> fits there. I don't understand why do you think it is hidden.
The function name doesn't make it obvious what it's doing. So all code
readers will have to follow the definition to see what it's doing. But
this point is not that important, so I won't argue.
> >>+ mtd->owner = pdev->dev.driver->owner;
> >>+ mtd->dev.parent = &pdev->dev;
> >>+ mtd->name = pdev->dev.driver->name;
> >
> >I think this is a little dangerous. You're potentially clobbering the
> >name that a driver already chose here. And why did you pick to use the
> >driver name? This gives non-unique names if there is more than one
> >device instantiated for a driver. That's why some drivers already use
> >the device name, not the driver name:
> >
> > mtd->name = dev_name(&pev->dev);
> >
> >And in fact, if any drivers are missing mtd->name, perhaps it's best to
> >just modify the MTD registration to give them a default:
> >
> > if (!mtd->name)
> > mtd->name = dev_name(&pdev->dev);
> >
> >>+}
>
> I don't clobber any name. The name is set as default before drivers
> might do this.
At the moment this is true, but the ordering is somewhat subtle if you
don't examine the details of mtd_setup_common_members() to see that it
is assigning a name. I can easily imagine some new driver in which
someone does:
mtd->name = ...;
[...]
mtd_setup_common_members(mtd, priv, pdev);
And doesn't notice that the ordering matters.
You could make this simpler by either
(1) making mtd_setup_common_members() check for mtd->name==NULL first
(2) just move the (default) name assignment to the common MTD
registration, like I suggested
> >>+
> >> extern int mtd_device_parse_register(struct mtd_info *mtd,
> >> const char * const *part_probe_types,
> >> struct mtd_part_parser_data *parser_data,
> >
> >How about we rethink the "helper" approach, and instead just do
> >validation in the core code? This would cover most of the important
> >parts of your helper, I think:
>
> Feel free to change all drivers. I like my approach with fixing 21
> bugs by reducing code by 44 lines.
I appreciate the bug fixes. I am just questioning the approach. Reduced
(source) code doesn't help anyone if it makes the code less
maintainable. My suggestions can probably make this more maintainable,
fix the same bugs, and reduce the source by a similar (albeit smaller)
number of lines.
> And it offers a common function where future similiarities can be
> put into too. Of course you can just add 21 lines, but that is not
> how I do such stuff.
I don't see how your common location helps much more than putting it in
mtdcore.c like I suggested, without the extra function.
> And I did the patches. If you don't like them, feel free to ignore
> them. I'm not playing remote keyboard but I do patches like I would
> do them, not like some arbitrary maintainer would do them. Sorry for
> the harsh words.
Arbitrary maintainer, eh? I'm simply suggesting that this could be
accomplished in a better way. If you don't want to take part in the
review process, then I have no obligation to engage either.
Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/27] mtd: nand: introduce function to fix a common bug in most nand-drivers not showing a device in sysfs
Am 28.05.2014 10:43, schrieb Brian Norris:
On Tue, May 27, 2014 at 12:12:26AM +0200, Alexander Holler wrote:
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -23,7 +23,7 @@
#include
#include
#include
-#include
+#include
#include
@@ -366,6 +366,15 @@ static inline int mtd_can_have_bb(const struct mtd_info
*mtd)
struct mtd_partition;
struct mtd_part_parser_data;
+static inline void mtd_setup_common_members(struct mtd_info *mtd, void *priv,
+ struct platform_device *pdev)
Thanks for the diligence on catching these issues, but I'm not sure this
helper function is fully the correct approach here.
+{
+ mtd->priv= priv;
I don't think you should hide this one here. It will be quite obvious if
a driver didn't stash its private data but tries to access it later. Are
there any drivers that missed this?
No, it just saves a line of source in all drivers and I think it fits
there. I don't understand why do you think it is hidden.
+ mtd->owner = pdev->dev.driver->owner;
+ mtd->dev.parent = &pdev->dev;
+ mtd->name= pdev->dev.driver->name;
I think this is a little dangerous. You're potentially clobbering the
name that a driver already chose here. And why did you pick to use the
driver name? This gives non-unique names if there is more than one
device instantiated for a driver. That's why some drivers already use
the device name, not the driver name:
mtd->name = dev_name(&pev->dev);
And in fact, if any drivers are missing mtd->name, perhaps it's best to
just modify the MTD registration to give them a default:
if (!mtd->name)
mtd->name = dev_name(&pdev->dev);
+}
I don't clobber any name. The name is set as default before drivers
might do this. And the common pattern I've seen wasn't dev_name(foo) but
the drivers name. And those drivers which do use dev_name(), still do so
by overwriting the default I put into that function. But feel free to
change this. I will not go again and again through the 26 drivers until
all maintainers and other people are happy.
BTW, nothing in this function actually makes sense to require a
platform_device, does it? And it's possible to have non-platform drivers
that want to do basic MTD initialization. So (if we still keep this
helper function at all), I'd recommend just a 'struct device *dev'
parameter.
Feel free to chgange it.
+
extern int mtd_device_parse_register(struct mtd_info *mtd,
const char * const *part_probe_types,
struct mtd_part_parser_data *parser_data,
How about we rethink the "helper" approach, and instead just do
validation in the core code? This would cover most of the important
parts of your helper, I think:
Feel free to change all drivers. I like my approach with fixing 21 bugs
by reducing code by 44 lines.
And it offers a common function where future similiarities can be put
into too. Of course you can just add 21 lines, but that is not how I do
such stuff.
And I did the patches. If you don't like them, feel free to ignore them.
I'm not playing remote keyboard but I do patches like I would do them,
not like some arbitrary maintainer would do them. Sorry for the harsh words.
Regards,
Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/27] mtd: nand: introduce function to fix a common bug in most nand-drivers not showing a device in sysfs
On Tue, May 27, 2014 at 12:12:26AM +0200, Alexander Holler wrote:
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -23,7 +23,7 @@
> #include
> #include
> #include
> -#include
> +#include
>
> #include
>
> @@ -366,6 +366,15 @@ static inline int mtd_can_have_bb(const struct mtd_info
> *mtd)
> struct mtd_partition;
> struct mtd_part_parser_data;
>
> +static inline void mtd_setup_common_members(struct mtd_info *mtd, void *priv,
> + struct platform_device *pdev)
Thanks for the diligence on catching these issues, but I'm not sure this
helper function is fully the correct approach here.
> +{
> + mtd->priv = priv;
I don't think you should hide this one here. It will be quite obvious if
a driver didn't stash its private data but tries to access it later. Are
there any drivers that missed this?
> + mtd->owner = pdev->dev.driver->owner;
> + mtd->dev.parent = &pdev->dev;
> + mtd->name = pdev->dev.driver->name;
I think this is a little dangerous. You're potentially clobbering the
name that a driver already chose here. And why did you pick to use the
driver name? This gives non-unique names if there is more than one
device instantiated for a driver. That's why some drivers already use
the device name, not the driver name:
mtd->name = dev_name(&pev->dev);
And in fact, if any drivers are missing mtd->name, perhaps it's best to
just modify the MTD registration to give them a default:
if (!mtd->name)
mtd->name = dev_name(&pdev->dev);
> +}
BTW, nothing in this function actually makes sense to require a
platform_device, does it? And it's possible to have non-platform drivers
that want to do basic MTD initialization. So (if we still keep this
helper function at all), I'd recommend just a 'struct device *dev'
parameter.
> +
> extern int mtd_device_parse_register(struct mtd_info *mtd,
>const char * const *part_probe_types,
>struct mtd_part_parser_data *parser_data,
How about we rethink the "helper" approach, and instead just do
validation in the core code? This would cover most of the important
parts of your helper, I think:
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index d201feeb3ca6..39ba5812a5a3 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -397,6 +397,11 @@ int add_mtd_device(struct mtd_info *mtd)
if (device_register(&mtd->dev) != 0)
goto fail_added;
+ if (mtd->dev.parent)
+ mtd->owner = mtd->dev.parent->driver->owner;
+ else
+ WARN_ON(1);
+
if (MTD_DEVT(i))
device_create(&mtd_class, mtd->dev.parent,
MTD_DEVT(i) + 1,
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 1ca9aec141ff..9869bbef50cf 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -370,7 +370,6 @@ static struct mtd_part *allocate_partition(struct mtd_info
*master,
slave->mtd.subpage_sft = master->subpage_sft;
slave->mtd.name = name;
- slave->mtd.owner = master->owner;
slave->mtd.backing_dev_info = master->backing_dev_info;
/* NOTE: we don't arrange MTDs as a tree; it'd be error-prone
--
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
