All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Frank Rowand <frowand.list@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] of: Split up name & type in modalias generation
Date: Mon, 10 Sep 2018 11:08:20 +0200	[thread overview]
Message-ID: <20180910090820.GB702@ulmo> (raw)
In-Reply-To: <32ff0b7e-a710-2c23-75ad-8c527d3453c7@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2000 bytes --]

On Fri, Sep 07, 2018 at 12:56:09PM -0700, Frank Rowand wrote:
> On 09/07/18 11:49, Rob Herring wrote:
> > On Fri, Sep 7, 2018 at 9:22 AM Thierry Reding <thierry.reding@gmail.com> wrote:
> >>
> >> From: Thierry Reding <treding@nvidia.com>
> >>
> >> The kernel's vsnprintf() implementation discards all alpha-numeric
> >> characters following a %p conversion specifier. This is done in order to
> >> generically skip any of the various modifiers that the kernel supports.
> >> Unfortunately, the OF modalias is generated with a format string that
> >> violates the assumption made by vsnprintf():
> >>
> >>         of:N%pOFnT%s
> >>
> >> While processing the above format string, vsnprintf() will eat the 'T'
> >> character, assuming that it belongs to the preceeding %p specifier. This
> >> results in a modalias with an incompatible format, which in turn causes
> >> the automatic loading of drivers based on modalias to no longer work.
> >>
> >> To fix this, split up the generation of the name & type fields into two
> >> separate snprintf() calls to avoid confusing the parser.
> >>
> >> Fixes: 73813f8483b1 ("of: Convert to using %pOFn instead of device_node.name")
> >> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >> ---
> >> Note that a more elegant fix would be to make the %p format specifier
> >> parser report back the exact number of characters consumed. I briefly
> >> tried to implement it, but quickly ran into numerous special cases
> >> that make this solution rather involved.
> >>
> >> I can spend some more time to improve this in general if that's what we
> >> ultimately want, but I think this patch is a better short-term fix to
> >> workaround the issue.
> > 
> > See my reply on the original patch. I've updated the patch in my
> > dt/next branch with the fix to use %c.
> > 
> > Rob
> > 
> 
> Agreed, your updated patch is more compact and looks cleaner.

Not sure about "cleaner", but yeah, that works as well.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

      reply	other threads:[~2018-09-10  9:08 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-07 14:22 [PATCH] of: Split up name & type in modalias generation Thierry Reding
2018-09-07 18:35 ` Frank Rowand
2018-09-07 18:49 ` Rob Herring
2018-09-07 19:56   ` Frank Rowand
2018-09-10  9:08     ` Thierry Reding [this message]

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=20180910090820.GB702@ulmo \
    --to=thierry.reding@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=frowand.list@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh+dt@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.