| 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
| ||
|
Message-ID: <7e96dd60-8f72-48f9-a393-5a8a7e5c6b18@bootlin.com>
Date: Fri, 3 Jan 2025 11:28:30 +0100
From: Thomas Richard <thomas.richard@...tlin.com>
To: Linus Walleij <linus.walleij@...aro.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Geert Uytterhoeven <geert+renesas@...der.be>
Cc: Bartosz Golaszewski <brgl@...ev.pl>, Lee Jones <lee@...nel.org>,
Pavel Machek <pavel@....cz>, linux-gpio@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org,
thomas.petazzoni@...tlin.com, DanieleCleri@...on.eu, GaryWang@...on.com.tw
Subject: Re: [PATCH 4/5] pinctrl: Add pin controller driver for AAEON UP
boards
On 12/22/24 00:43, Linus Walleij wrote:
> Hi Thomas,
>
> thanks for your detailed reply!
>
> On Fri, Dec 20, 2024 at 2:50 PM Thomas Richard
> <thomas.richard@...tlin.com> wrote:
>
>> Yes my cover letter was a bit short, and maybe some context was missing.
>
> The text and graphics below explain it very well, so please include them
> into the commit message so we have it there!
>
>> This FPGA acts as a level shifter between the Intel SoC pins and the pin
>> header, and also makes a kind of switch/mux.
>
> Since it's Intel we need to notify Andy to help out with this so that
> it gets done in a way that works with how he think consumers
> should interact with Intel pin control and GPIO.
>
>> +---------+ +--------------+ +---+
>> | | | | H |
>> |---------| |-------------| E |
>> | | | | A |
>> Intel Soc |---------| FPGA |-------------| D |
>> | | | | E |
>> |---------| |-------------| R |
>> | | | | |
>> ----------+ +--------------+ +---+
>>
>>
>> For most of the pins, the FPGA opens/closes a switch to enable/disable
>> the access to the SoC pin from a pin header.
>> Each "switch", has a direction flag that shall be set in tandem with the
>> status of the SoC pin.
>> For example, if the SoC pin is in PWM mode, the "switch" shall be
>> configured in output direction.
>> If the SoC pin is set in GPIO mode, the direction of the "switch" shall
>> corresponds to the GPIO direction.
>>
>> +---------+ +--------------+ +---+
>> | | | | H |
>> | | \ | | E |
>> | PWM1 | \ | | A |
>> Intel Soc |--------------|----- \-----|-------------| D |
>> | | | | E |
>> | | | | R |
>> | | FPGA | | |
>> ----------+ +--------------+ +---+
>>
>> (PWM1 pin from Intel SoC can be used as PWM, and also in GPIO mode,
>> thanks to the Intel pinctrl driver).
>>
>>
>> Few pins (PINMUX_* pins) work differently. The FPGA acts as a mux and
>> routes for example the I2C0_SDA pin or GPIOX (of the SoC) to the pin header.
>>
>> +---------+ +--------------+ +---+
>> | I2C0_SDA | | | H |
>> |-----------|----- \ | | E |
>> | | \ | | A |
>> Intel Soc | | \-----|-------------| D |
>> | GPIOX | | | E |
>> |-----------|----- | | R |
>> | | FPGA | | |
>> ----------+ +--------------+ +---+
>>
>> The pin header looks like this:
>> +--------------------+--------------------+
>> | 3.3V | 5V |
>> | GPIO2 / I2C1_SDA | 5V |
>> | GPIO3 / I2C1_SCL | GND |
>> | GPIO4 / ADC0 | GPIO14 / UART1_TX |
>> | GND | GPIO15 / UART1_RX |
>> | GPIO17 / UART1_RTS | GPIO18 / I2S_CLK |
>> | GPIO27 | GND |
>> | GPIO22 | GPIO23 |
>> | 3.3V | GPIO24 |
>> | GPIO10 / SPI_MOSI | GND |
>> | GPIO9 / SPI_MISO | GPIO25 |
>> | GPIO11 / SPI_CLK | GPIO8 / SPI_CS0 |
>> | GND | GPIO7 / SPI_CS1 |
>> | GPIO0 / I2C0_SDA | GPIO1 / I2C0_SCL |
>> | GPIO5 | GND |
>> | GPIO6 | GPIO12 / PWM0 |
>> | GPIO13 / PWM1 | GND |
>> | GPIO19 / I2S_FRM | GPIO16 / UART1_CTS |
>> | GPIO26 | GPIO20 / I2S_DIN |
>> | GND | GPIO21 / I2S_DOUT |
>> +--------------------+--------------------+
>>
>> The GPIOs in the pin header corresponds to the gpiochip I declare in
>> this driver.
>> So when I want to use a pin in GPIO mode, the upboard pinctrl driver
>> requests the corresponding SoC GPIO to the Intel pinctrl driver.
>> The SoC pins connected to the FPGA, are identified with "external" id.
>>
>> The hardware and the FPGA were designed in tandem, so you know for
>> example that for the GPIOX you need to request the Nth "external" GPIO.
>>
>> When you drive your GPIO, the upboard gpiochip manages in the same time
>> the direction of the "switch" and the value/direction of the
>> corresponding SoC pin.
>>
>> +------------------+ +--------------+ +---+
>> |---------| |-------------| H |
>> |---------| GPIOCHIP |-------------| E |
>> Intel gpiochip |---------| |-------------| A |
>> provided by Intel |---------| FPGA |-------------| D |
>> pinctrl driver |---------| |-------------| E |
>> |---------| |-------------| R |
>> |---------| |-------------| |
>> +------------------+ +--------------+ +---+
>>
>>
>> About gpiochip_add_pinlist_range(), I added it because the FPGA pins
>> used by the gpiochip are not consecutive.
>>
>> Please let me know if it is not clear.
>> And sorry I'm not very good to make ascii art.
>
> I get it! We have a similar driver in the kernel already, look into:
> drivers/gpio/gpio-aggregator.c
>
> The aggregator abstraction is however just software. What you
> need here is a gpio-aggregator that adds some hardware
> control on top. But it has a very nice design using a bitmap
> to keep track of the GPIOs etc, and it supports operations
> on multiple GPIOs (many man-hours of hard coding and
> design went into that driver, ask Geert and Andy...)
>
> So I would proceed like this:
>
> - The pin control part of the driver looks sound, except
> for the way you add ranges.
>
> - The gpiochip part needs to be refactored using the
> ideas from gpio-aggregator.c.
>
> - Look closely at aggregator and see what you can do
> based on that code, if you can mimic how it picks up
> and forwards all GPIO functions. Maybe part of it
> needs to be made into a library?
> <linux/gpio/gpio-aggregator.h>?
> For example if you start to feel like "I would really like
> to just call gpio_fwd_get_multiple() then this is what
> you want to do. The library can probably still be
> inside gpio-aggregator.c the way we do it in
> e.g. gpio-mmio.c, just export and keep library functions
> separately.
Hi Linus,
Ok I think I understand what you expect.
I started to look at the gpio-aggregator code, play a bit with it, and
refactor it to use it from my driver.
My main issue is about the request of the SoC GPIOs done by the aggregator.
If from my driver I call the aggregator library to create a gpiochip,
the SoC pins will be requested. So the SoC pins will be set in GPIO
mode, and the pins will never be in function mode.
There is no way to set the pins back to function mode (even if the GPIO
is free).
I tried to add a feature in the aggregator to defer the request of the gpio.
So at the beginning of each ops the gpio_desc is checked. If it is
valid, the gpio can be used. Otherwise, the gpio is requested.
For example:
gpio_fwd_get() {
if (!gpio_desc_is_valid(desc))
desc = request_gpio()
return gpiod_get_value(desc)
}
But when a gpiochip is registered, the core calls get_direction() or
direction_input(), so all GPIOs are requested and it does not solve my
problem.
I expect to register a gpiochip without setting all pins in GPIO mode at
probe time (like all pinctrl driver do).
But I did not find a solution.
Best Regards,
Thomas
Powered by blists - more mailing lists