linux-kernel - Re: [PATCH v3 3/3] pwm: core: add consumer device link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190215102248.i2mggrw74lgs2owq@pengutronix.de>
Date:   Fri, 15 Feb 2019 11:22:48 +0100
From:   Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
To:     Claudiu.Beznea@...rochip.com
Cc:     fabrice.gasnier@...com, thierry.reding@...il.com,
        robh+dt@...nel.org, mark.rutland@....com,
        devicetree@...r.kernel.org, alexandre.torgue@...com,
        tduszyns@...il.com, linux-kernel@...r.kernel.org,
        linux-pwm@...r.kernel.org, mcoquelin.stm32@...il.com,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 3/3] pwm: core: add consumer device link

On Fri, Feb 15, 2019 at 09:23:48AM +0000, Claudiu.Beznea@...rochip.com wrote:
> 
> 
> On 13.02.2019 12:50, Fabrice Gasnier wrote:
> > Add a device link between the PWM consumer and the PWM provider. This
> > enforces the PWM user to get suspended before the PWM provider. It
> > allows proper synchronization of suspend/resume sequences: the PWM user
> > is responsible for properly stopping PWM, before the provider gets
> > suspended: see [1]. Add the device link in:
> > - of_pwm_get()
> > - pwm_get()
> > - devm_ variants
> > as it requires a reference to the device for the PWM consumer.
> > 
> > [1] https://lkml.org/lkml/2019/2/5/770
> > 
> > Suggested-by: Thierry Reding <thierry.reding@...il.com>
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@...com>
> > ---
> > Changes in v3:
> > - add struct device to of_get_pwm() arguments to handle device link from
> >   there.
> > ---
> >  drivers/pwm/core.c  | 14 +++++++++++---
> >  include/linux/pwm.h |  6 ++++--
> >  2 files changed, 15 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> > index 1581f6a..8cb5d4bc 100644
> > --- a/drivers/pwm/core.c
> > +++ b/drivers/pwm/core.c
> > @@ -638,6 +638,7 @@ static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
> >  
> >  /**
> >   * of_pwm_get() - request a PWM via the PWM framework
> > + * @dev: device for PWM consumer
> >   * @np: device node to get the PWM from
> >   * @con_id: consumer name
> >   *
> > @@ -655,7 +656,8 @@ static struct pwm_chip *of_node_to_pwmchip(struct device_node *np)
> >   * Returns: A pointer to the requested PWM device or an ERR_PTR()-encoded
> >   * error code on failure.
> >   */
> > -struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id)
> > +struct pwm_device *of_pwm_get(struct device *dev, struct device_node *np,
> > +			      const char *con_id)
> >  {
> >  	struct pwm_device *pwm = NULL;
> >  	struct of_phandle_args args;
> > @@ -689,6 +691,9 @@ struct pwm_device *of_pwm_get(struct device_node *np, const char *con_id)
> >  	if (IS_ERR(pwm))
> >  		goto put;
> >  
> > +	if (!device_link_add(dev, pwm->chip->dev, DL_FLAG_AUTOREMOVE_CONSUMER))
> > +		pr_debug("%s(): device link not added\n", __func__);
> > +
> 
> Just asking... will this mechanism also be working for PWMs requested via
> sysfs? At a first look it seems not.

No, but I would not expect a problem because AFAIU userspace is
suspended before kernel devices. However as userspace might not notice
the suspend it probably fails to stop the pwm and so will likely prevent
the suspend.

Best regards
Uwe

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