All of lore.kernel.org
 help / color / mirror / Atom feed
From: Javier Martinez Canillas <javier@osg.samsung.com>
To: Brian Norris <computersforpeace@gmail.com>
Cc: Mark Brown <broonie@kernel.org>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	linux-mtd@lists.infradead.org,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org
Subject: Re: spi: OF module autoloading is still broken
Date: Wed, 18 Nov 2015 17:07:14 -0300	[thread overview]
Message-ID: <564CDA72.8030800@osg.samsung.com> (raw)
In-Reply-To: <564A4B66.6090107@osg.samsung.com>

Hello Brian and Mark,

On 11/16/2015 06:32 PM, Javier Martinez Canillas wrote:
> On 11/16/2015 05:47 PM, Brian Norris wrote:
>> On Mon, Nov 16, 2015 at 05:00:43PM -0300, Javier Martinez Canillas wrote:
>>> On 11/16/2015 04:24 PM, Brian Norris wrote:

[snip]

>>>> I don't think you have problems only with bad FDTs. I think you have a
>>>> problem with perfectly valid DTs.
>>>>
>>>
>>> Agreed, I wonder if spi_uevent() shouldn't be changed to report both the
>>> OF and old SPI modaliases to avoid breaking a lot of drivers but at the
>>> same time allowing DT-only drivers to not need an SPI ID table.
>>
>> That's the solution I imagined, though I haven't tested it yet. I don't
>> see much precedent for reporting multiple modaliases, so there could be
>> some kind of ABI issues around that too.
>>
> 
> Ok, I'm glad that we agree. This definitely needs to be discussed with more
> people. I'll re-spin my patch and post a v2 reporting multiple modaliases
> tomorrow after testing.
> 

So I had some time today to test this approach but unfortunately it seems
this workaround will not be possible because AFAICT kmod only takes into
account the last MODALIAS reported as a uevent. I still have to check the
kmod source code but that's my conclusion from trying to report both aliases.

When looking at the uevent sysfs entry for the device, I see that both aliases
are reported but if only a OF alias is built into the module, then kmod does
not auto load the module unless the OF MODALIAS is the last one reported, i.e:

# grep MODALIAS /sys/devices/platform/12d40000.spi/spi_master/spi2/spi2.0/uevent
MODALIAS=spi:cros-ec-spi
MODALIAS=of:Ncros-ecT<NULL>Cgoogle,cros-ec-spi

# modinfo cros_ec_spi | grep alias
alias:          of:N*T*Cgoogle,cros-ec-spi*

If I invert the order on which the uevent are reported, then the module is
not autoloaded, i.e:

# grep MODALIAS /sys/devices/platform/12d40000.spi/spi_master/spi2/spi2.0/uevent
MODALIAS=of:Ncros-ecT<NULL>Cgoogle,cros-ec-spi
MODALIAS=spi:cros-ec-spi

# modinfo cros_ec_spi | grep alias
alias:          of:N*T*Cgoogle,cros-ec-spi*

In this case only is loaded if the module has a spi:<alias>, i.e:

# modinfo cros_ec_spi | grep alias
alias:          of:N*T*Cgoogle,cros-ec-spi*
alias:          spi:cros-ec-spi

IOW, even when is possible to report more than one MODALIAS, user-space seems
to only use the last one reported so using both don't work.

Of course kmod can be changed to check for more than one MODALIAS but since
the kernel should not break old user-space, the fact that a single MODALIAS
was reported seems to be an ABI now due how is used.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

WARNING: multiple messages have this Message-ID (diff)
From: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
To: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Heiner Kallweit
	<hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Dmitry Torokhov
	<dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: spi: OF module autoloading is still broken
Date: Wed, 18 Nov 2015 17:07:14 -0300	[thread overview]
Message-ID: <564CDA72.8030800@osg.samsung.com> (raw)
In-Reply-To: <564A4B66.6090107-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Hello Brian and Mark,

On 11/16/2015 06:32 PM, Javier Martinez Canillas wrote:
> On 11/16/2015 05:47 PM, Brian Norris wrote:
>> On Mon, Nov 16, 2015 at 05:00:43PM -0300, Javier Martinez Canillas wrote:
>>> On 11/16/2015 04:24 PM, Brian Norris wrote:

[snip]

>>>> I don't think you have problems only with bad FDTs. I think you have a
>>>> problem with perfectly valid DTs.
>>>>
>>>
>>> Agreed, I wonder if spi_uevent() shouldn't be changed to report both the
>>> OF and old SPI modaliases to avoid breaking a lot of drivers but at the
>>> same time allowing DT-only drivers to not need an SPI ID table.
>>
>> That's the solution I imagined, though I haven't tested it yet. I don't
>> see much precedent for reporting multiple modaliases, so there could be
>> some kind of ABI issues around that too.
>>
> 
> Ok, I'm glad that we agree. This definitely needs to be discussed with more
> people. I'll re-spin my patch and post a v2 reporting multiple modaliases
> tomorrow after testing.
> 

So I had some time today to test this approach but unfortunately it seems
this workaround will not be possible because AFAICT kmod only takes into
account the last MODALIAS reported as a uevent. I still have to check the
kmod source code but that's my conclusion from trying to report both aliases.

When looking at the uevent sysfs entry for the device, I see that both aliases
are reported but if only a OF alias is built into the module, then kmod does
not auto load the module unless the OF MODALIAS is the last one reported, i.e:

# grep MODALIAS /sys/devices/platform/12d40000.spi/spi_master/spi2/spi2.0/uevent
MODALIAS=spi:cros-ec-spi
MODALIAS=of:Ncros-ecT<NULL>Cgoogle,cros-ec-spi

# modinfo cros_ec_spi | grep alias
alias:          of:N*T*Cgoogle,cros-ec-spi*

If I invert the order on which the uevent are reported, then the module is
not autoloaded, i.e:

# grep MODALIAS /sys/devices/platform/12d40000.spi/spi_master/spi2/spi2.0/uevent
MODALIAS=of:Ncros-ecT<NULL>Cgoogle,cros-ec-spi
MODALIAS=spi:cros-ec-spi

# modinfo cros_ec_spi | grep alias
alias:          of:N*T*Cgoogle,cros-ec-spi*

In this case only is loaded if the module has a spi:<alias>, i.e:

# modinfo cros_ec_spi | grep alias
alias:          of:N*T*Cgoogle,cros-ec-spi*
alias:          spi:cros-ec-spi

IOW, even when is possible to report more than one MODALIAS, user-space seems
to only use the last one reported so using both don't work.

Of course kmod can be changed to check for more than one MODALIAS but since
the kernel should not break old user-space, the fact that a single MODALIAS
was reported seems to be an ABI now due how is used.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
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

  parent reply	other threads:[~2015-11-18 20:07 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-03 21:54 m25p80: Commit "allow arbitrary OF matching for "jedec,spi-nor"" breaks module autoloading Heiner Kallweit
2015-11-12 18:59 ` m25p80: Commit "allow arbitrary OF matching for "jedec, spi-nor"" " Brian Norris
2015-11-12 18:59   ` m25p80: Commit "allow arbitrary OF matching for "jedec,spi-nor"" " Brian Norris
2015-11-12 18:59   ` Brian Norris
2015-11-13 19:40   ` spi: OF module autoloading is still broken (was: Re: m25p80: Commit "allow arbitrary OF matching for "jedec,spi-nor"" breaks module autoloading) Brian Norris
2015-11-13 19:40     ` Brian Norris
2015-11-13 22:12     ` Mark Brown
2015-11-13 22:12       ` Mark Brown
2015-11-13 22:51       ` Brian Norris
2015-11-13 23:14         ` Mark Brown
2015-11-13 23:14           ` Mark Brown
2015-11-13 23:48           ` Brian Norris
2015-11-13 23:48             ` Brian Norris
2015-11-16 13:53             ` Mark Brown
2015-11-16 13:53               ` Mark Brown
2015-11-16 17:26               ` spi: OF module autoloading is still broken Javier Martinez Canillas
2015-11-16 17:26                 ` Javier Martinez Canillas
2015-11-16 17:51                 ` Mark Brown
2015-11-16 17:51                   ` Mark Brown
2015-11-16 18:00                   ` Javier Martinez Canillas
2015-11-16 18:00                     ` Javier Martinez Canillas
2015-11-16 17:19             ` Javier Martinez Canillas
2015-11-16 17:19               ` Javier Martinez Canillas
2015-11-16 17:49               ` Mark Brown
2015-11-16 17:49                 ` Mark Brown
2015-11-16 17:57                 ` Javier Martinez Canillas
2015-11-16 17:57                   ` Javier Martinez Canillas
2015-11-16 19:24               ` Brian Norris
2015-11-16 19:24                 ` Brian Norris
2015-11-16 20:00                 ` Javier Martinez Canillas
2015-11-16 20:00                   ` Javier Martinez Canillas
2015-11-16 20:47                   ` Brian Norris
2015-11-16 20:47                     ` Brian Norris
2015-11-16 21:32                     ` Javier Martinez Canillas
2015-11-16 21:51                       ` Brian Norris
2015-11-16 21:51                         ` Brian Norris
2015-11-17 13:14                         ` Javier Martinez Canillas
2015-11-17 13:14                           ` Javier Martinez Canillas
2015-11-17 13:19                           ` Mark Brown
2015-11-17 13:19                             ` Mark Brown
2015-11-17 13:36                             ` Javier Martinez Canillas
2015-11-18 20:07                       ` Javier Martinez Canillas [this message]
2015-11-18 20:07                         ` Javier Martinez Canillas
2015-11-19 12:47                         ` Javier Martinez Canillas
2015-11-17 13:34                   ` Mark Brown
2015-11-17 13:34                     ` Mark Brown
2015-11-17 13:38                     ` Javier Martinez Canillas
2015-11-17 13:38                       ` Javier Martinez Canillas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=564CDA72.8030800@osg.samsung.com \
    --to=javier@osg.samsung.com \
    --cc=broonie@kernel.org \
    --cc=computersforpeace@gmail.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.