linux-kernel - Re: [PATCH v2 2/8] pinctrl: stm32: Introduce HDP driver

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: <94795d0c-0c73-41eb-ada6-9a01b2ac5892@foss.st.com>
Date: Wed, 21 May 2025 11:49:47 +0200
From: Clement LE GOFFIC <clement.legoffic@...s.st.com>
To: Linus Walleij <linus.walleij@...aro.org>
CC: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Maxime Coquelin
	<mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Bartosz Golaszewski <brgl@...ev.pl>, <linux-kernel@...r.kernel.org>,
        <linux-gpio@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-stm32@...md-mailman.stormreply.com>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 2/8] pinctrl: stm32: Introduce HDP driver

On 5/21/25 00:34, Linus Walleij wrote:
> Hi Clément,
> 
> thanks for your patch!
> 
> On Tue, May 20, 2025 at 5:04 PM Clément Le Goffic
> <clement.legoffic@...s.st.com> wrote:
> 
>> This patch introduce the driver for the Hardware Debug Port available on
>> STM32MP platforms. The HDP allows the observation of internal SoC
>> signals by using multiplexers. Each HDP port can provide up to 16
>> internal signals (one of them can be software controlled as a GPO).
>>
>> Signed-off-by: Clément Le Goffic <clement.legoffic@...s.st.com>
> 
> (...)
>> +static int stm32_hdp_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>> +{
>> +       return GPIO_LINE_DIRECTION_OUT;
>> +}
> 
> That's reasonable.
> 
>> +static int stm32_hdp_gpio_get(struct gpio_chip *gc, unsigned int offset)
>> +{
>> +       struct stm32_hdp *hdp = gpiochip_get_data(gc);
>> +
>> +       if (((hdp->mux_conf & HDP_MUX_MASK(offset))) == HDP_MUX_GPOVAL(offset))
>> +               return !!(readl_relaxed(hdp->base + HDP_GPOVAL) & BIT(offset));
>> +       else
>> +               return !!(readl_relaxed(hdp->base + HDP_VAL) & BIT(offset));
>> +}
> 
> ...but you still make it possible to read the value of the line
> if it's not muxed as GPO?
> 
> Should it not stm32_hdp_gpio_get_direction() return
> GPIO_LINE_DIRECTION_IN if HDP_MUX_MASK(offset))) != HDP_MUX_GPOVAL(offset)?

Hi, oops, you're right !

> 
>> +static void stm32_hdp_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
>> +{
>> +       struct stm32_hdp *hdp = gpiochip_get_data(gc);
>> +
>> +       if (value)
>> +               writel_relaxed(BIT(offset), hdp->base + HDP_GPOSET);
>> +       else
>> +               writel_relaxed(BIT(offset), hdp->base + HDP_GPOCLR);
>> +}
> 
> Can't you just use GPIO_GENERIC for this?
> 
> bgpio_init(gc, dev, ARRAY_SIZE(stm32_hdp_pins), // == 8
>      hdp->base + HDP_VAL,
>      hdp->base + HDP_GPOSET,
>      hdp->base + HDP_GPOCLR,
>      NULL,
>      NULL,
>      0);
> 
> The default behaviour of GPIO MMIO is to read the output register
> for the value if the line is in output mode.
> 
> You may wanna override the .get_direction() callback after bgpio_init()
> and before registering the chip, either with what you have or what
> I described above.

I didn't know about it, I'll take a look and provide a V3.

Thank you,

Clément

> Yours,
> Linus Walleij