* Autoloading of SPI/nor driver on kirkwood (qnap devices)
@ 2014-09-10 6:00 ` Brian Norris
0 siblings, 0 replies; 23+ messages in thread
From: Brian Norris @ 2014-09-10 6:00 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Sep 04, 2014 at 09:02:04AM +0200, Geert Uytterhoeven wrote:
> On Thu, Sep 4, 2014 at 4:11 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> >> I noticed that many platforms declare the flash chip as compatible =
> >> "st,m25pXXX" whereas the ts219.dtsi just said m25p128 but I tried
> >> changing that and it didn't help. In any case without spi-nor.ko being
> >> autoloaded I don't support m25p80.ko ever would be.
> >
> > m25p80.c has:
> >
> > static struct spi_driver m25p80_driver = {
> > ...
> > .id_table = spi_nor_ids,
> > ...
> > };
> >
> > while spi_nor_ids is defined in spi-nor.c. Since they end up as two
> > separate modules, modpost can't read the ID table and add the device ID
> > aliases to m25p80.ko.
>
> Woops. This indeed doesn't work.
> Note that the MODULE_DEVICE_TABLE() is also gone from m25p80.c
>
> So m25p80.c needs to contain the IDs, while spi-nor.c also needs the IDs
> because it's a framework/library.
Actually, m25p80.c only needs the strings (i.e., the named aliases --
character data), and for the most part, spi-nor.c only needs the IDs (i.e.,
the identification bytes).
What's more, I don't think that any modern SPI NOR user really needs to
be specifying exactly which SPI device it is via a specific string. Our
driver code pretty much always re-detects the device by reading its
device ID. All the SPI NOR code needs to know is how to read its ID.
> A quick solution would be to move the ID table to a header file, and include
> that by both, and re-add the lost MODULE_DEVICE_TABLE() to m25p80.c.
> That duplicates the data, though.
>
> Hmm, for the built-in case, we can avoid the duplication by letting m25p80.c
> refer to the table in spi-nor.c if !MODULE.
> Does anyone see a better solution?
A little bit of a shot in the dark, as I haven't fleshed this one out:
Would it work to just copy the SPI ID strings into m25p80.c, keep the
full table in spi-nor.c, stop adding SPI ID string (and DT) bindings to
the m25p80 table (force platforms to use "m25p80"-compatible probing, or
maybe something generic like "spi-nor-rdid", "spi-nor-sfdp", etc.) and
eventually kill the strings from spi-nor.c entirely? I really don't want
to propagate string-binding much further into the SPI NOR library, since
everything should be auto-detectable--partly just by an ID table as we
have now, but eventually we can use CFI or SFDP parameters provided by
relatively new SPI NOR chips.
I'm not sure if this is great for the short-term problem of fixing
module-autoloading. Perhaps we should do a short-term hack to duplicate
the SPI ID strings to m25p80.c by including them in a header (and
backport for 3.16.y stable?), but I'd like to disentangle this.
Brian
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: Autoloading of SPI/nor driver on kirkwood (qnap devices)
@ 2014-09-10 6:00 ` Brian Norris
0 siblings, 0 replies; 23+ messages in thread
From: Brian Norris @ 2014-09-10 6:00 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Andrew Lunn, Jason Cooper, linux-spi, Huang Shijie,
MTD Maling List, Ian Campbell, Sebastian Hesselbarth,
Ben Hutchings, linux-arm-kernel, debian-kernel
On Thu, Sep 04, 2014 at 09:02:04AM +0200, Geert Uytterhoeven wrote:
> On Thu, Sep 4, 2014 at 4:11 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> >> I noticed that many platforms declare the flash chip as compatible =
> >> "st,m25pXXX" whereas the ts219.dtsi just said m25p128 but I tried
> >> changing that and it didn't help. In any case without spi-nor.ko being
> >> autoloaded I don't support m25p80.ko ever would be.
> >
> > m25p80.c has:
> >
> > static struct spi_driver m25p80_driver = {
> > ...
> > .id_table = spi_nor_ids,
> > ...
> > };
> >
> > while spi_nor_ids is defined in spi-nor.c. Since they end up as two
> > separate modules, modpost can't read the ID table and add the device ID
> > aliases to m25p80.ko.
>
> Woops. This indeed doesn't work.
> Note that the MODULE_DEVICE_TABLE() is also gone from m25p80.c
>
> So m25p80.c needs to contain the IDs, while spi-nor.c also needs the IDs
> because it's a framework/library.
Actually, m25p80.c only needs the strings (i.e., the named aliases --
character data), and for the most part, spi-nor.c only needs the IDs (i.e.,
the identification bytes).
What's more, I don't think that any modern SPI NOR user really needs to
be specifying exactly which SPI device it is via a specific string. Our
driver code pretty much always re-detects the device by reading its
device ID. All the SPI NOR code needs to know is how to read its ID.
> A quick solution would be to move the ID table to a header file, and include
> that by both, and re-add the lost MODULE_DEVICE_TABLE() to m25p80.c.
> That duplicates the data, though.
>
> Hmm, for the built-in case, we can avoid the duplication by letting m25p80.c
> refer to the table in spi-nor.c if !MODULE.
> Does anyone see a better solution?
A little bit of a shot in the dark, as I haven't fleshed this one out:
Would it work to just copy the SPI ID strings into m25p80.c, keep the
full table in spi-nor.c, stop adding SPI ID string (and DT) bindings to
the m25p80 table (force platforms to use "m25p80"-compatible probing, or
maybe something generic like "spi-nor-rdid", "spi-nor-sfdp", etc.) and
eventually kill the strings from spi-nor.c entirely? I really don't want
to propagate string-binding much further into the SPI NOR library, since
everything should be auto-detectable--partly just by an ID table as we
have now, but eventually we can use CFI or SFDP parameters provided by
relatively new SPI NOR chips.
I'm not sure if this is great for the short-term problem of fixing
module-autoloading. Perhaps we should do a short-term hack to duplicate
the SPI ID strings to m25p80.c by including them in a header (and
backport for 3.16.y stable?), but I'd like to disentangle this.
Brian
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: Autoloading of SPI/nor driver on kirkwood (qnap devices)
2014-09-10 6:00 ` Brian Norris
(?)
@ 2014-09-10 6:11 ` Brian Norris
-1 siblings, 0 replies; 23+ messages in thread
From: Brian Norris @ 2014-09-10 6:11 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Ben Hutchings, Andrew Lunn, Jason Cooper, linux-spi,
MTD Maling List, Ian Campbell, debian-kernel,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Sebastian Hesselbarth, Huang Shijie, Rafał Miłecki
+ Huang (his Freescale address is bouncing), Rafal
On Tue, Sep 9, 2014 at 11:00 PM, Brian Norris
<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Sep 04, 2014 at 09:02:04AM +0200, Geert Uytterhoeven wrote:
>> On Thu, Sep 4, 2014 at 4:11 AM, Ben Hutchings <ben-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org> wrote:
>> >> I noticed that many platforms declare the flash chip as compatible =
>> >> "st,m25pXXX" whereas the ts219.dtsi just said m25p128 but I tried
>> >> changing that and it didn't help. In any case without spi-nor.ko being
>> >> autoloaded I don't support m25p80.ko ever would be.
>> >
>> > m25p80.c has:
>> >
>> > static struct spi_driver m25p80_driver = {
>> > ...
>> > .id_table = spi_nor_ids,
>> > ...
>> > };
>> >
>> > while spi_nor_ids is defined in spi-nor.c. Since they end up as two
>> > separate modules, modpost can't read the ID table and add the device ID
>> > aliases to m25p80.ko.
>>
>> Woops. This indeed doesn't work.
>> Note that the MODULE_DEVICE_TABLE() is also gone from m25p80.c
>>
>> So m25p80.c needs to contain the IDs, while spi-nor.c also needs the IDs
>> because it's a framework/library.
>
> Actually, m25p80.c only needs the strings (i.e., the named aliases --
> character data), and for the most part, spi-nor.c only needs the IDs (i.e.,
> the identification bytes).
>
> What's more, I don't think that any modern SPI NOR user really needs to
> be specifying exactly which SPI device it is via a specific string. Our
> driver code pretty much always re-detects the device by reading its
> device ID. All the SPI NOR code needs to know is how to read its ID.
>
>> A quick solution would be to move the ID table to a header file, and include
>> that by both, and re-add the lost MODULE_DEVICE_TABLE() to m25p80.c.
>> That duplicates the data, though.
>>
>> Hmm, for the built-in case, we can avoid the duplication by letting m25p80.c
>> refer to the table in spi-nor.c if !MODULE.
>> Does anyone see a better solution?
>
> A little bit of a shot in the dark, as I haven't fleshed this one out:
>
> Would it work to just copy the SPI ID strings into m25p80.c, keep the
> full table in spi-nor.c, stop adding SPI ID string (and DT) bindings to
> the m25p80 table (force platforms to use "m25p80"-compatible probing, or
> maybe something generic like "spi-nor-rdid", "spi-nor-sfdp", etc.) and
> eventually kill the strings from spi-nor.c entirely? I really don't want
> to propagate string-binding much further into the SPI NOR library, since
> everything should be auto-detectable--partly just by an ID table as we
> have now, but eventually we can use CFI or SFDP parameters provided by
> relatively new SPI NOR chips.
>
> I'm not sure if this is great for the short-term problem of fixing
> module-autoloading. Perhaps we should do a short-term hack to duplicate
> the SPI ID strings to m25p80.c by including them in a header (and
> backport for 3.16.y stable?), but I'd like to disentangle this.
>
> Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread* Autoloading of SPI/nor driver on kirkwood (qnap devices)
@ 2014-09-10 6:11 ` Brian Norris
0 siblings, 0 replies; 23+ messages in thread
From: Brian Norris @ 2014-09-10 6:11 UTC (permalink / raw)
To: linux-arm-kernel
+ Huang (his Freescale address is bouncing), Rafal
On Tue, Sep 9, 2014 at 11:00 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Thu, Sep 04, 2014 at 09:02:04AM +0200, Geert Uytterhoeven wrote:
>> On Thu, Sep 4, 2014 at 4:11 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
>> >> I noticed that many platforms declare the flash chip as compatible =
>> >> "st,m25pXXX" whereas the ts219.dtsi just said m25p128 but I tried
>> >> changing that and it didn't help. In any case without spi-nor.ko being
>> >> autoloaded I don't support m25p80.ko ever would be.
>> >
>> > m25p80.c has:
>> >
>> > static struct spi_driver m25p80_driver = {
>> > ...
>> > .id_table = spi_nor_ids,
>> > ...
>> > };
>> >
>> > while spi_nor_ids is defined in spi-nor.c. Since they end up as two
>> > separate modules, modpost can't read the ID table and add the device ID
>> > aliases to m25p80.ko.
>>
>> Woops. This indeed doesn't work.
>> Note that the MODULE_DEVICE_TABLE() is also gone from m25p80.c
>>
>> So m25p80.c needs to contain the IDs, while spi-nor.c also needs the IDs
>> because it's a framework/library.
>
> Actually, m25p80.c only needs the strings (i.e., the named aliases --
> character data), and for the most part, spi-nor.c only needs the IDs (i.e.,
> the identification bytes).
>
> What's more, I don't think that any modern SPI NOR user really needs to
> be specifying exactly which SPI device it is via a specific string. Our
> driver code pretty much always re-detects the device by reading its
> device ID. All the SPI NOR code needs to know is how to read its ID.
>
>> A quick solution would be to move the ID table to a header file, and include
>> that by both, and re-add the lost MODULE_DEVICE_TABLE() to m25p80.c.
>> That duplicates the data, though.
>>
>> Hmm, for the built-in case, we can avoid the duplication by letting m25p80.c
>> refer to the table in spi-nor.c if !MODULE.
>> Does anyone see a better solution?
>
> A little bit of a shot in the dark, as I haven't fleshed this one out:
>
> Would it work to just copy the SPI ID strings into m25p80.c, keep the
> full table in spi-nor.c, stop adding SPI ID string (and DT) bindings to
> the m25p80 table (force platforms to use "m25p80"-compatible probing, or
> maybe something generic like "spi-nor-rdid", "spi-nor-sfdp", etc.) and
> eventually kill the strings from spi-nor.c entirely? I really don't want
> to propagate string-binding much further into the SPI NOR library, since
> everything should be auto-detectable--partly just by an ID table as we
> have now, but eventually we can use CFI or SFDP parameters provided by
> relatively new SPI NOR chips.
>
> I'm not sure if this is great for the short-term problem of fixing
> module-autoloading. Perhaps we should do a short-term hack to duplicate
> the SPI ID strings to m25p80.c by including them in a header (and
> backport for 3.16.y stable?), but I'd like to disentangle this.
>
> Brian
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: Autoloading of SPI/nor driver on kirkwood (qnap devices)
@ 2014-09-10 6:11 ` Brian Norris
0 siblings, 0 replies; 23+ messages in thread
From: Brian Norris @ 2014-09-10 6:11 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Andrew Lunn, Jason Cooper, Huang Shijie, Rafał Miłecki,
linux-spi, MTD Maling List, Ian Campbell, Sebastian Hesselbarth,
Ben Hutchings, linux-arm-kernel, debian-kernel
+ Huang (his Freescale address is bouncing), Rafal
On Tue, Sep 9, 2014 at 11:00 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Thu, Sep 04, 2014 at 09:02:04AM +0200, Geert Uytterhoeven wrote:
>> On Thu, Sep 4, 2014 at 4:11 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
>> >> I noticed that many platforms declare the flash chip as compatible =
>> >> "st,m25pXXX" whereas the ts219.dtsi just said m25p128 but I tried
>> >> changing that and it didn't help. In any case without spi-nor.ko being
>> >> autoloaded I don't support m25p80.ko ever would be.
>> >
>> > m25p80.c has:
>> >
>> > static struct spi_driver m25p80_driver = {
>> > ...
>> > .id_table = spi_nor_ids,
>> > ...
>> > };
>> >
>> > while spi_nor_ids is defined in spi-nor.c. Since they end up as two
>> > separate modules, modpost can't read the ID table and add the device ID
>> > aliases to m25p80.ko.
>>
>> Woops. This indeed doesn't work.
>> Note that the MODULE_DEVICE_TABLE() is also gone from m25p80.c
>>
>> So m25p80.c needs to contain the IDs, while spi-nor.c also needs the IDs
>> because it's a framework/library.
>
> Actually, m25p80.c only needs the strings (i.e., the named aliases --
> character data), and for the most part, spi-nor.c only needs the IDs (i.e.,
> the identification bytes).
>
> What's more, I don't think that any modern SPI NOR user really needs to
> be specifying exactly which SPI device it is via a specific string. Our
> driver code pretty much always re-detects the device by reading its
> device ID. All the SPI NOR code needs to know is how to read its ID.
>
>> A quick solution would be to move the ID table to a header file, and include
>> that by both, and re-add the lost MODULE_DEVICE_TABLE() to m25p80.c.
>> That duplicates the data, though.
>>
>> Hmm, for the built-in case, we can avoid the duplication by letting m25p80.c
>> refer to the table in spi-nor.c if !MODULE.
>> Does anyone see a better solution?
>
> A little bit of a shot in the dark, as I haven't fleshed this one out:
>
> Would it work to just copy the SPI ID strings into m25p80.c, keep the
> full table in spi-nor.c, stop adding SPI ID string (and DT) bindings to
> the m25p80 table (force platforms to use "m25p80"-compatible probing, or
> maybe something generic like "spi-nor-rdid", "spi-nor-sfdp", etc.) and
> eventually kill the strings from spi-nor.c entirely? I really don't want
> to propagate string-binding much further into the SPI NOR library, since
> everything should be auto-detectable--partly just by an ID table as we
> have now, but eventually we can use CFI or SFDP parameters provided by
> relatively new SPI NOR chips.
>
> I'm not sure if this is great for the short-term problem of fixing
> module-autoloading. Perhaps we should do a short-term hack to duplicate
> the SPI ID strings to m25p80.c by including them in a header (and
> backport for 3.16.y stable?), but I'd like to disentangle this.
>
> Brian
^ permalink raw reply [flat|nested] 23+ messages in thread[parent not found: <CAN8TOE_BRyg=Ww0oOxGeRr5Xkwh5hTq0UKQAR2dFjRVfwcBZoQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: Autoloading of SPI/nor driver on kirkwood (qnap devices)
2014-09-10 6:11 ` Brian Norris
(?)
@ 2014-09-10 7:59 ` Geert Uytterhoeven
-1 siblings, 0 replies; 23+ messages in thread
From: Geert Uytterhoeven @ 2014-09-10 7:59 UTC (permalink / raw)
To: Brian Norris
Cc: Ben Hutchings, Andrew Lunn, Jason Cooper, linux-spi,
MTD Maling List, Ian Campbell, debian-kernel,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Sebastian Hesselbarth, Huang Shijie, Rafał Miłecki,
devicetree-u79uwXL29TY76Z2rM5mHXA
+ devicetree, as the names are used in DTS file.
On Wed, Sep 10, 2014 at 8:11 AM, Brian Norris
<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> + Huang (his Freescale address is bouncing), Rafal
>
> On Tue, Sep 9, 2014 at 11:00 PM, Brian Norris
> <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Thu, Sep 04, 2014 at 09:02:04AM +0200, Geert Uytterhoeven wrote:
>>> On Thu, Sep 4, 2014 at 4:11 AM, Ben Hutchings <ben-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org> wrote:
>>> >> I noticed that many platforms declare the flash chip as compatible =
>>> >> "st,m25pXXX" whereas the ts219.dtsi just said m25p128 but I tried
>>> >> changing that and it didn't help. In any case without spi-nor.ko being
>>> >> autoloaded I don't support m25p80.ko ever would be.
>>> >
>>> > m25p80.c has:
>>> >
>>> > static struct spi_driver m25p80_driver = {
>>> > ...
>>> > .id_table = spi_nor_ids,
>>> > ...
>>> > };
>>> >
>>> > while spi_nor_ids is defined in spi-nor.c. Since they end up as two
>>> > separate modules, modpost can't read the ID table and add the device ID
>>> > aliases to m25p80.ko.
>>>
>>> Woops. This indeed doesn't work.
>>> Note that the MODULE_DEVICE_TABLE() is also gone from m25p80.c
>>>
>>> So m25p80.c needs to contain the IDs, while spi-nor.c also needs the IDs
>>> because it's a framework/library.
>>
>> Actually, m25p80.c only needs the strings (i.e., the named aliases --
>> character data), and for the most part, spi-nor.c only needs the IDs (i.e.,
>> the identification bytes).
>>
>> What's more, I don't think that any modern SPI NOR user really needs to
>> be specifying exactly which SPI device it is via a specific string. Our
>> driver code pretty much always re-detects the device by reading its
>> device ID. All the SPI NOR code needs to know is how to read its ID.
But the proper name should be in the compatible property.
>>> A quick solution would be to move the ID table to a header file, and include
>>> that by both, and re-add the lost MODULE_DEVICE_TABLE() to m25p80.c.
>>> That duplicates the data, though.
>>>
>>> Hmm, for the built-in case, we can avoid the duplication by letting m25p80.c
>>> refer to the table in spi-nor.c if !MODULE.
>>> Does anyone see a better solution?
>>
>> A little bit of a shot in the dark, as I haven't fleshed this one out:
>>
>> Would it work to just copy the SPI ID strings into m25p80.c, keep the
>> full table in spi-nor.c, stop adding SPI ID string (and DT) bindings to
>> the m25p80 table (force platforms to use "m25p80"-compatible probing, or
>> maybe something generic like "spi-nor-rdid", "spi-nor-sfdp", etc.) and
>> eventually kill the strings from spi-nor.c entirely? I really don't want
>> to propagate string-binding much further into the SPI NOR library, since
>> everything should be auto-detectable--partly just by an ID table as we
>> have now, but eventually we can use CFI or SFDP parameters provided by
>> relatively new SPI NOR chips.
How does the SPI core know to use m25p80? From the ids, and the compatible
value. If you won't add new names, all devices should declare compatiblity
with "m25p80" (that's "spansion,m25p80", right?) as a fallback.
However, are all devices really compatible with m25p80?
I.e. does m25p80.c work will all devices, in the absense of support for the
real device? If not, the fallback must not be there, and new device names
must be added.
>> I'm not sure if this is great for the short-term problem of fixing
>> module-autoloading. Perhaps we should do a short-term hack to duplicate
>> the SPI ID strings to m25p80.c by including them in a header (and
>> backport for 3.16.y stable?), but I'd like to disentangle this.
>>
>> Brian
Autoloading is based on "spi:<name>". If new names aren't added, m25p80
won't be loaded for new SPI FLASHes, unless I'm missing something?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 23+ messages in thread* Autoloading of SPI/nor driver on kirkwood (qnap devices)
@ 2014-09-10 7:59 ` Geert Uytterhoeven
0 siblings, 0 replies; 23+ messages in thread
From: Geert Uytterhoeven @ 2014-09-10 7:59 UTC (permalink / raw)
To: linux-arm-kernel
+ devicetree, as the names are used in DTS file.
On Wed, Sep 10, 2014 at 8:11 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> + Huang (his Freescale address is bouncing), Rafal
>
> On Tue, Sep 9, 2014 at 11:00 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
>> On Thu, Sep 04, 2014 at 09:02:04AM +0200, Geert Uytterhoeven wrote:
>>> On Thu, Sep 4, 2014 at 4:11 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
>>> >> I noticed that many platforms declare the flash chip as compatible =
>>> >> "st,m25pXXX" whereas the ts219.dtsi just said m25p128 but I tried
>>> >> changing that and it didn't help. In any case without spi-nor.ko being
>>> >> autoloaded I don't support m25p80.ko ever would be.
>>> >
>>> > m25p80.c has:
>>> >
>>> > static struct spi_driver m25p80_driver = {
>>> > ...
>>> > .id_table = spi_nor_ids,
>>> > ...
>>> > };
>>> >
>>> > while spi_nor_ids is defined in spi-nor.c. Since they end up as two
>>> > separate modules, modpost can't read the ID table and add the device ID
>>> > aliases to m25p80.ko.
>>>
>>> Woops. This indeed doesn't work.
>>> Note that the MODULE_DEVICE_TABLE() is also gone from m25p80.c
>>>
>>> So m25p80.c needs to contain the IDs, while spi-nor.c also needs the IDs
>>> because it's a framework/library.
>>
>> Actually, m25p80.c only needs the strings (i.e., the named aliases --
>> character data), and for the most part, spi-nor.c only needs the IDs (i.e.,
>> the identification bytes).
>>
>> What's more, I don't think that any modern SPI NOR user really needs to
>> be specifying exactly which SPI device it is via a specific string. Our
>> driver code pretty much always re-detects the device by reading its
>> device ID. All the SPI NOR code needs to know is how to read its ID.
But the proper name should be in the compatible property.
>>> A quick solution would be to move the ID table to a header file, and include
>>> that by both, and re-add the lost MODULE_DEVICE_TABLE() to m25p80.c.
>>> That duplicates the data, though.
>>>
>>> Hmm, for the built-in case, we can avoid the duplication by letting m25p80.c
>>> refer to the table in spi-nor.c if !MODULE.
>>> Does anyone see a better solution?
>>
>> A little bit of a shot in the dark, as I haven't fleshed this one out:
>>
>> Would it work to just copy the SPI ID strings into m25p80.c, keep the
>> full table in spi-nor.c, stop adding SPI ID string (and DT) bindings to
>> the m25p80 table (force platforms to use "m25p80"-compatible probing, or
>> maybe something generic like "spi-nor-rdid", "spi-nor-sfdp", etc.) and
>> eventually kill the strings from spi-nor.c entirely? I really don't want
>> to propagate string-binding much further into the SPI NOR library, since
>> everything should be auto-detectable--partly just by an ID table as we
>> have now, but eventually we can use CFI or SFDP parameters provided by
>> relatively new SPI NOR chips.
How does the SPI core know to use m25p80? From the ids, and the compatible
value. If you won't add new names, all devices should declare compatiblity
with "m25p80" (that's "spansion,m25p80", right?) as a fallback.
However, are all devices really compatible with m25p80?
I.e. does m25p80.c work will all devices, in the absense of support for the
real device? If not, the fallback must not be there, and new device names
must be added.
>> I'm not sure if this is great for the short-term problem of fixing
>> module-autoloading. Perhaps we should do a short-term hack to duplicate
>> the SPI ID strings to m25p80.c by including them in a header (and
>> backport for 3.16.y stable?), but I'd like to disentangle this.
>>
>> Brian
Autoloading is based on "spi:<name>". If new names aren't added, m25p80
won't be loaded for new SPI FLASHes, unless I'm missing something?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: Autoloading of SPI/nor driver on kirkwood (qnap devices)
@ 2014-09-10 7:59 ` Geert Uytterhoeven
0 siblings, 0 replies; 23+ messages in thread
From: Geert Uytterhoeven @ 2014-09-10 7:59 UTC (permalink / raw)
To: Brian Norris
Cc: Andrew Lunn, Jason Cooper, devicetree, Huang Shijie,
Rafał Miłecki, linux-spi, MTD Maling List,
Ian Campbell, Sebastian Hesselbarth, Ben Hutchings,
linux-arm-kernel, debian-kernel
+ devicetree, as the names are used in DTS file.
On Wed, Sep 10, 2014 at 8:11 AM, Brian Norris
<computersforpeace@gmail.com> wrote:
> + Huang (his Freescale address is bouncing), Rafal
>
> On Tue, Sep 9, 2014 at 11:00 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
>> On Thu, Sep 04, 2014 at 09:02:04AM +0200, Geert Uytterhoeven wrote:
>>> On Thu, Sep 4, 2014 at 4:11 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
>>> >> I noticed that many platforms declare the flash chip as compatible =
>>> >> "st,m25pXXX" whereas the ts219.dtsi just said m25p128 but I tried
>>> >> changing that and it didn't help. In any case without spi-nor.ko being
>>> >> autoloaded I don't support m25p80.ko ever would be.
>>> >
>>> > m25p80.c has:
>>> >
>>> > static struct spi_driver m25p80_driver = {
>>> > ...
>>> > .id_table = spi_nor_ids,
>>> > ...
>>> > };
>>> >
>>> > while spi_nor_ids is defined in spi-nor.c. Since they end up as two
>>> > separate modules, modpost can't read the ID table and add the device ID
>>> > aliases to m25p80.ko.
>>>
>>> Woops. This indeed doesn't work.
>>> Note that the MODULE_DEVICE_TABLE() is also gone from m25p80.c
>>>
>>> So m25p80.c needs to contain the IDs, while spi-nor.c also needs the IDs
>>> because it's a framework/library.
>>
>> Actually, m25p80.c only needs the strings (i.e., the named aliases --
>> character data), and for the most part, spi-nor.c only needs the IDs (i.e.,
>> the identification bytes).
>>
>> What's more, I don't think that any modern SPI NOR user really needs to
>> be specifying exactly which SPI device it is via a specific string. Our
>> driver code pretty much always re-detects the device by reading its
>> device ID. All the SPI NOR code needs to know is how to read its ID.
But the proper name should be in the compatible property.
>>> A quick solution would be to move the ID table to a header file, and include
>>> that by both, and re-add the lost MODULE_DEVICE_TABLE() to m25p80.c.
>>> That duplicates the data, though.
>>>
>>> Hmm, for the built-in case, we can avoid the duplication by letting m25p80.c
>>> refer to the table in spi-nor.c if !MODULE.
>>> Does anyone see a better solution?
>>
>> A little bit of a shot in the dark, as I haven't fleshed this one out:
>>
>> Would it work to just copy the SPI ID strings into m25p80.c, keep the
>> full table in spi-nor.c, stop adding SPI ID string (and DT) bindings to
>> the m25p80 table (force platforms to use "m25p80"-compatible probing, or
>> maybe something generic like "spi-nor-rdid", "spi-nor-sfdp", etc.) and
>> eventually kill the strings from spi-nor.c entirely? I really don't want
>> to propagate string-binding much further into the SPI NOR library, since
>> everything should be auto-detectable--partly just by an ID table as we
>> have now, but eventually we can use CFI or SFDP parameters provided by
>> relatively new SPI NOR chips.
How does the SPI core know to use m25p80? From the ids, and the compatible
value. If you won't add new names, all devices should declare compatiblity
with "m25p80" (that's "spansion,m25p80", right?) as a fallback.
However, are all devices really compatible with m25p80?
I.e. does m25p80.c work will all devices, in the absense of support for the
real device? If not, the fallback must not be there, and new device names
must be added.
>> I'm not sure if this is great for the short-term problem of fixing
>> module-autoloading. Perhaps we should do a short-term hack to duplicate
>> the SPI ID strings to m25p80.c by including them in a header (and
>> backport for 3.16.y stable?), but I'd like to disentangle this.
>>
>> Brian
Autoloading is based on "spi:<name>". If new names aren't added, m25p80
won't be loaded for new SPI FLASHes, unless I'm missing something?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: Autoloading of SPI/nor driver on kirkwood (qnap devices)
2014-09-10 6:00 ` Brian Norris
(?)
@ 2014-09-14 2:00 ` Ben Hutchings
-1 siblings, 0 replies; 23+ messages in thread
From: Ben Hutchings @ 2014-09-14 2:00 UTC (permalink / raw)
To: Brian Norris
Cc: Geert Uytterhoeven, Andrew Lunn, Jason Cooper, linux-spi,
Huang Shijie, MTD Maling List, Ian Campbell, debian-kernel,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Sebastian Hesselbarth
[-- Attachment #1: Type: text/plain, Size: 3917 bytes --]
On Tue, 2014-09-09 at 23:00 -0700, Brian Norris wrote:
> On Thu, Sep 04, 2014 at 09:02:04AM +0200, Geert Uytterhoeven wrote:
> > On Thu, Sep 4, 2014 at 4:11 AM, Ben Hutchings <ben-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org> wrote:
> > >> I noticed that many platforms declare the flash chip as compatible =
> > >> "st,m25pXXX" whereas the ts219.dtsi just said m25p128 but I tried
> > >> changing that and it didn't help. In any case without spi-nor.ko being
> > >> autoloaded I don't support m25p80.ko ever would be.
> > >
> > > m25p80.c has:
> > >
> > > static struct spi_driver m25p80_driver = {
> > > ...
> > > .id_table = spi_nor_ids,
> > > ...
> > > };
> > >
> > > while spi_nor_ids is defined in spi-nor.c. Since they end up as two
> > > separate modules, modpost can't read the ID table and add the device ID
> > > aliases to m25p80.ko.
> >
> > Woops. This indeed doesn't work.
> > Note that the MODULE_DEVICE_TABLE() is also gone from m25p80.c
> >
> > So m25p80.c needs to contain the IDs, while spi-nor.c also needs the IDs
> > because it's a framework/library.
>
> Actually, m25p80.c only needs the strings (i.e., the named aliases --
> character data), and for the most part, spi-nor.c only needs the IDs (i.e.,
> the identification bytes).
Unfortunately, spi-nor.c also needs the strings to implement
spi_nor_match_id() (which is only used by fsl-quadspi.c) and to report
mismatches in spi_nor_scan().
But spi_nor_match_id(), spi_nor_scan() and spi_nor::read_id should work
with something like struct flash_info, not struct spi_device_id.
> What's more, I don't think that any modern SPI NOR user really needs to
> be specifying exactly which SPI device it is via a specific string. Our
> driver code pretty much always re-detects the device by reading its
> device ID. All the SPI NOR code needs to know is how to read its ID.
>
> > A quick solution would be to move the ID table to a header file, and include
> > that by both, and re-add the lost MODULE_DEVICE_TABLE() to m25p80.c.
> > That duplicates the data, though.
> >
> > Hmm, for the built-in case, we can avoid the duplication by letting m25p80.c
> > refer to the table in spi-nor.c if !MODULE.
> > Does anyone see a better solution?
>
> A little bit of a shot in the dark, as I haven't fleshed this one out:
>
> Would it work to just copy the SPI ID strings into m25p80.c, keep the
> full table in spi-nor.c, stop adding SPI ID string (and DT) bindings to
> the m25p80 table (force platforms to use "m25p80"-compatible probing, or
> maybe something generic like "spi-nor-rdid", "spi-nor-sfdp", etc.) and
> eventually kill the strings from spi-nor.c entirely?
I think that a DT node is always supposed to include a compatible string
for the specific device. It could also include a generic compatible
string for SPI NOR chips, but the *only* thing a driver can do with that
is to use the JEDEC ID command. It can't even generically read a single
byte, since addresses may be either 3 or 4 bytes long! So I think that
if a generic compatible string is defined, the DT binding must also
define the properties that spi-nor currently reads out of its static
table.
> I really don't want
> to propagate string-binding much further into the SPI NOR library, since
> everything should be auto-detectable--partly just by an ID table as we
> have now, but eventually we can use CFI or SFDP parameters provided by
> relatively new SPI NOR chips.
>
> I'm not sure if this is great for the short-term problem of fixing
> module-autoloading. Perhaps we should do a short-term hack to duplicate
> the SPI ID strings to m25p80.c by including them in a header (and
> backport for 3.16.y stable?), but I'd like to disentangle this.
I'll have a go at doing that.
Ben.
--
Ben Hutchings
The world is coming to an end. Please log off.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* Autoloading of SPI/nor driver on kirkwood (qnap devices)
@ 2014-09-14 2:00 ` Ben Hutchings
0 siblings, 0 replies; 23+ messages in thread
From: Ben Hutchings @ 2014-09-14 2:00 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2014-09-09 at 23:00 -0700, Brian Norris wrote:
> On Thu, Sep 04, 2014 at 09:02:04AM +0200, Geert Uytterhoeven wrote:
> > On Thu, Sep 4, 2014 at 4:11 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> > >> I noticed that many platforms declare the flash chip as compatible =
> > >> "st,m25pXXX" whereas the ts219.dtsi just said m25p128 but I tried
> > >> changing that and it didn't help. In any case without spi-nor.ko being
> > >> autoloaded I don't support m25p80.ko ever would be.
> > >
> > > m25p80.c has:
> > >
> > > static struct spi_driver m25p80_driver = {
> > > ...
> > > .id_table = spi_nor_ids,
> > > ...
> > > };
> > >
> > > while spi_nor_ids is defined in spi-nor.c. Since they end up as two
> > > separate modules, modpost can't read the ID table and add the device ID
> > > aliases to m25p80.ko.
> >
> > Woops. This indeed doesn't work.
> > Note that the MODULE_DEVICE_TABLE() is also gone from m25p80.c
> >
> > So m25p80.c needs to contain the IDs, while spi-nor.c also needs the IDs
> > because it's a framework/library.
>
> Actually, m25p80.c only needs the strings (i.e., the named aliases --
> character data), and for the most part, spi-nor.c only needs the IDs (i.e.,
> the identification bytes).
Unfortunately, spi-nor.c also needs the strings to implement
spi_nor_match_id() (which is only used by fsl-quadspi.c) and to report
mismatches in spi_nor_scan().
But spi_nor_match_id(), spi_nor_scan() and spi_nor::read_id should work
with something like struct flash_info, not struct spi_device_id.
> What's more, I don't think that any modern SPI NOR user really needs to
> be specifying exactly which SPI device it is via a specific string. Our
> driver code pretty much always re-detects the device by reading its
> device ID. All the SPI NOR code needs to know is how to read its ID.
>
> > A quick solution would be to move the ID table to a header file, and include
> > that by both, and re-add the lost MODULE_DEVICE_TABLE() to m25p80.c.
> > That duplicates the data, though.
> >
> > Hmm, for the built-in case, we can avoid the duplication by letting m25p80.c
> > refer to the table in spi-nor.c if !MODULE.
> > Does anyone see a better solution?
>
> A little bit of a shot in the dark, as I haven't fleshed this one out:
>
> Would it work to just copy the SPI ID strings into m25p80.c, keep the
> full table in spi-nor.c, stop adding SPI ID string (and DT) bindings to
> the m25p80 table (force platforms to use "m25p80"-compatible probing, or
> maybe something generic like "spi-nor-rdid", "spi-nor-sfdp", etc.) and
> eventually kill the strings from spi-nor.c entirely?
I think that a DT node is always supposed to include a compatible string
for the specific device. It could also include a generic compatible
string for SPI NOR chips, but the *only* thing a driver can do with that
is to use the JEDEC ID command. It can't even generically read a single
byte, since addresses may be either 3 or 4 bytes long! So I think that
if a generic compatible string is defined, the DT binding must also
define the properties that spi-nor currently reads out of its static
table.
> I really don't want
> to propagate string-binding much further into the SPI NOR library, since
> everything should be auto-detectable--partly just by an ID table as we
> have now, but eventually we can use CFI or SFDP parameters provided by
> relatively new SPI NOR chips.
>
> I'm not sure if this is great for the short-term problem of fixing
> module-autoloading. Perhaps we should do a short-term hack to duplicate
> the SPI ID strings to m25p80.c by including them in a header (and
> backport for 3.16.y stable?), but I'd like to disentangle this.
I'll have a go at doing that.
Ben.
--
Ben Hutchings
The world is coming to an end. Please log off.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 811 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140914/4dc0dc57/attachment.sig>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: Autoloading of SPI/nor driver on kirkwood (qnap devices)
@ 2014-09-14 2:00 ` Ben Hutchings
0 siblings, 0 replies; 23+ messages in thread
From: Ben Hutchings @ 2014-09-14 2:00 UTC (permalink / raw)
To: Brian Norris
Cc: Andrew Lunn, Jason Cooper, linux-spi, Huang Shijie,
Geert Uytterhoeven, Ian Campbell, MTD Maling List,
Sebastian Hesselbarth, linux-arm-kernel, debian-kernel
[-- Attachment #1: Type: text/plain, Size: 3893 bytes --]
On Tue, 2014-09-09 at 23:00 -0700, Brian Norris wrote:
> On Thu, Sep 04, 2014 at 09:02:04AM +0200, Geert Uytterhoeven wrote:
> > On Thu, Sep 4, 2014 at 4:11 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> > >> I noticed that many platforms declare the flash chip as compatible =
> > >> "st,m25pXXX" whereas the ts219.dtsi just said m25p128 but I tried
> > >> changing that and it didn't help. In any case without spi-nor.ko being
> > >> autoloaded I don't support m25p80.ko ever would be.
> > >
> > > m25p80.c has:
> > >
> > > static struct spi_driver m25p80_driver = {
> > > ...
> > > .id_table = spi_nor_ids,
> > > ...
> > > };
> > >
> > > while spi_nor_ids is defined in spi-nor.c. Since they end up as two
> > > separate modules, modpost can't read the ID table and add the device ID
> > > aliases to m25p80.ko.
> >
> > Woops. This indeed doesn't work.
> > Note that the MODULE_DEVICE_TABLE() is also gone from m25p80.c
> >
> > So m25p80.c needs to contain the IDs, while spi-nor.c also needs the IDs
> > because it's a framework/library.
>
> Actually, m25p80.c only needs the strings (i.e., the named aliases --
> character data), and for the most part, spi-nor.c only needs the IDs (i.e.,
> the identification bytes).
Unfortunately, spi-nor.c also needs the strings to implement
spi_nor_match_id() (which is only used by fsl-quadspi.c) and to report
mismatches in spi_nor_scan().
But spi_nor_match_id(), spi_nor_scan() and spi_nor::read_id should work
with something like struct flash_info, not struct spi_device_id.
> What's more, I don't think that any modern SPI NOR user really needs to
> be specifying exactly which SPI device it is via a specific string. Our
> driver code pretty much always re-detects the device by reading its
> device ID. All the SPI NOR code needs to know is how to read its ID.
>
> > A quick solution would be to move the ID table to a header file, and include
> > that by both, and re-add the lost MODULE_DEVICE_TABLE() to m25p80.c.
> > That duplicates the data, though.
> >
> > Hmm, for the built-in case, we can avoid the duplication by letting m25p80.c
> > refer to the table in spi-nor.c if !MODULE.
> > Does anyone see a better solution?
>
> A little bit of a shot in the dark, as I haven't fleshed this one out:
>
> Would it work to just copy the SPI ID strings into m25p80.c, keep the
> full table in spi-nor.c, stop adding SPI ID string (and DT) bindings to
> the m25p80 table (force platforms to use "m25p80"-compatible probing, or
> maybe something generic like "spi-nor-rdid", "spi-nor-sfdp", etc.) and
> eventually kill the strings from spi-nor.c entirely?
I think that a DT node is always supposed to include a compatible string
for the specific device. It could also include a generic compatible
string for SPI NOR chips, but the *only* thing a driver can do with that
is to use the JEDEC ID command. It can't even generically read a single
byte, since addresses may be either 3 or 4 bytes long! So I think that
if a generic compatible string is defined, the DT binding must also
define the properties that spi-nor currently reads out of its static
table.
> I really don't want
> to propagate string-binding much further into the SPI NOR library, since
> everything should be auto-detectable--partly just by an ID table as we
> have now, but eventually we can use CFI or SFDP parameters provided by
> relatively new SPI NOR chips.
>
> I'm not sure if this is great for the short-term problem of fixing
> module-autoloading. Perhaps we should do a short-term hack to duplicate
> the SPI ID strings to m25p80.c by including them in a header (and
> backport for 3.16.y stable?), but I'd like to disentangle this.
I'll have a go at doing that.
Ben.
--
Ben Hutchings
The world is coming to an end. Please log off.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread