All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Michael Walle <michael@walle.cc>,
	Thierry Reding <thierry.reding@gmail.com>,
	Lee Jones <lee.jones@linaro.org>
Cc: linux-pwm@vger.kernel.org, kernel@pengutronix.de
Subject: Re: [PATCH] pwm: sl28cpld: fix getting driver data in pwm callbacks
Date: Mon, 30 Nov 2020 15:58:05 +0100	[thread overview]
Message-ID: <20201130145805.n5il3xsrsifuomcu@pengutronix.de> (raw)
In-Reply-To: <3becef43bd9119c624cf6c3c8ee5b472@walle.cc>

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

Hello Michael,

On Mon, Nov 30, 2020 at 09:17:52AM +0100, Michael Walle wrote:
> Am 2020-11-24 22:24, schrieb Uwe Kleine-König:
> > Currently .get_state() and .apply() use dev_get_drvdata() on the struct
> > device related to the pwm chip. This only works after .probe() called
> > platform_set_drvdata() which in this driver happens only after
> > pwmchip_add() and so comes possibly too late.
> > 
> > Instead of setting the driver data earlier use the traditional
> > container_of approach as this way the driver data is conceptually and
> > computational nearer.
> > 
> > Fixes: 9db33d221efc ("pwm: Add support for sl28cpld PWM controller")
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > in v4 of the driver Michael still used container_of and then changed to
> > use dev_get_drvdata() as Lee suggested. I didn't notice this suggestion,
> > otherwise I would have expressed my variance about this already earlier.
> > 
> > I noticed this problem because Michael contacted me via irc and showed
> > me the resulting oops, so I think applying this before 5.10 would be
> > good. It's not entirely clear to me why .get_state() is called that
> > early in his case, but the theory is clear: The callbacks can be called
> > as soon as pwmchip_add() is called.
> > 
> > Best regards
> > Uwe
> 
> Ping. Would be nice if this makes it into v5.10.

Yes, fully agreed. @Thierry: Do you care to send it to Linus? Or are you
ok if I do that?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

      reply	other threads:[~2020-11-30 14:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-24 21:24 [PATCH] pwm: sl28cpld: fix getting driver data in pwm callbacks Uwe Kleine-König
2020-11-24 22:22 ` Michael Walle
2020-11-25  6:48   ` Uwe Kleine-König
2020-11-30  8:17 ` Michael Walle
2020-11-30 14:58   ` Uwe Kleine-König [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=20201130145805.n5il3xsrsifuomcu@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=kernel@pengutronix.de \
    --cc=lee.jones@linaro.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=michael@walle.cc \
    --cc=thierry.reding@gmail.com \
    /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.