linux-kernel - Re: [PATCH v2 6/6] pinctrl: intel: Enumerate PWM device when community has a capabilitty

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y2t5ZXM0Oihz/LDK@smile.fi.intel.com>
Date:   Wed, 9 Nov 2022 11:56:53 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Thierry Reding <thierry.reding@...il.com>,
        linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
        linux-pwm@...r.kernel.org
Subject: Re: [PATCH v2 6/6] pinctrl: intel: Enumerate PWM device when
 community has a capabilitty

On Wed, Nov 09, 2022 at 10:08:51AM +0100, Linus Walleij wrote:
> On Tue, Nov 8, 2022 at 3:22 PM Andy Shevchenko
> <andriy.shevchenko@...ux.intel.com> wrote:

...

> > +       static const struct pwm_lpss_boardinfo info = {
> > +               .clk_rate = 19200000,
> > +               .npwm = 1,
> > +               .base_unit_bits = 22,
> > +               .bypass = true,
> > +       };
> > +       struct pwm_lpss_chip *pwm;
> > +
> > +       if (!(community->features & PINCTRL_FEATURE_PWM))
> > +               return 0;
> > +
> > +       pwm = pwm_lpss_probe(pctrl->dev, community->regs + PWMC, &info);
> > +       if (IS_ERR(pwm) && PTR_ERR(pwm) != -ENODEV)
> > +               return PTR_ERR(pwm);
> 
> This is alike a boardfile embedded into the pin control driver.

Correct.

> It's a bit backwards since we usually go the other direction, i.e. probe
> a PWM driver or whatever and then that driver request its resources
> that are assigned from e.g. DT or ACPI, and in this case that would
> mean it request its pin control handle and the pins get set up.
> 
> I guess I can be convinced that this hack is the lesser evil :D
> 
> What is it in the platform that makes this kind of hacks necessary?

The PWM capability is discoverable by the looking for it in the pin
control IP MMIO, it's not a separate device, but a sibling (child?)
of the pin control, that's not a separate entity.

Moreover, not every pin control _community_ has that capability (capabilities
are on the Community level and depends on ACPI representation of the
communities themself - single device or device per community - the PWM may or
may not be easily attached.

What you are proposing is to invent at least two additional properties or so
for the pin control device description and then to support old platforms,
create a board file somewhere else, which will go through all pin control
devices, checks the capabilities, then embeds the properties via properties
(Either embedded into DSDT, if done in BIOS, or swnodes).

Do I get you right?

If so, in my opinion it's way more ugly and overkill that the current
approach.

> Inconsistent description in ACPI or is the PWM device simply
> missing from the DSDT (or whatever is the right form in ACPI)
> in already shipped devices that need it?

Right.

> Or is it simply impossible to describe the PWM device in ACPI?

It's a dynamic feature and existing firmwares are carved in stone.
It might be possible to move the above mentioned uglification to
the BIOS. But the horizon of that is 5+ years in case I am able
to convince program managers for it (TBH, I don't believe it's
feasible, since "Windows works!" mantra, they are not engineers).

That said, I agree that this looks not nice, but that's all what
Mika and me can come up with to make all this as little ugly and
intrusive as possible.

-- 
With Best Regards,
Andy Shevchenko