| 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
| ||
|
Date: Thu, 27 Jun 2024 22:46:15 +0530
From: Ayush Singh <ayush@...gleboard.org>
To: Andrew Davis <afd@...com>, Mark Brown <broonie@...nel.org>,
Vaishnav M A <vaishnav@...gleboard.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Derek Kiernan <derek.kiernan@....com>,
Dragan Cvetic <dragan.cvetic@....com>, Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Nishanth Menon <nm@...com>,
Vignesh Raghavendra <vigneshr@...com>, Tero Kristo <kristo@...nel.org>,
Michael Walle <mwalle@...nel.org>, Andrew Lunn <andrew@...n.ch>,
jkridner@...gleboard.org, robertcnelson@...gleboard.org
Cc: linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v5 7/7] dts: ti: k3-am625-beagleplay: Add mikroBUS
On 6/27/24 22:37, Andrew Davis wrote:
> On 6/27/24 11:26 AM, Ayush Singh wrote:
>> DONOTMERGE
>>
>> Add mikroBUS connector and some mikroBUS boards support for Beagleplay.
>> The mikroBUS boards node should probably be moved to a more appropriate
>> location but I am not quite sure where it should go since it is not
>> dependent on specific arch.
>>
>> Signed-off-by: Ayush Singh <ayush@...gleboard.org>
>> ---
>> arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 94
>> +++++++++++++++++++++++---
>> 1 file changed, 86 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>> b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>> index 70de288d728e..3f3cd70345c4 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
>> @@ -38,6 +38,7 @@ aliases {
>> serial2 = &main_uart0;
>> usb0 = &usb0;
>> usb1 = &usb1;
>> + mikrobus0 = &mikrobus0;
>> };
>> chosen {
>> @@ -227,6 +228,56 @@ simple-audio-card,codec {
>> };
>> };
>> + mikrobus0: mikrobus-connector {
>> + compatible = "mikrobus-connector";
>> + pinctrl-names = "default", "pwm_default", "pwm_gpio",
>> + "uart_default", "uart_gpio", "i2c_default",
>> + "i2c_gpio", "spi_default", "spi_gpio";
>> + pinctrl-0 = <&mikrobus_gpio_pins_default>;
>> + pinctrl-1 = <&mikrobus_pwm_pins_default>;
>> + pinctrl-2 = <&mikrobus_pwm_pins_gpio>;
>> + pinctrl-3 = <&mikrobus_uart_pins_default>;
>> + pinctrl-4 = <&mikrobus_uart_pins_gpio>;
>> + pinctrl-5 = <&mikrobus_i2c_pins_default>;
>> + pinctrl-6 = <&mikrobus_i2c_pins_gpio>;
>> + pinctrl-7 = <&mikrobus_spi_pins_default>;
>> + pinctrl-8 = <&mikrobus_spi_pins_gpio>;
>> +
>> + mikrobus-gpio-names = "pwm", "int", "rx", "tx", "scl", "sda",
>> + "mosi", "miso", "sck", "cs", "rst", "an";
>> + mikrobus-gpios = <&main_gpio1 11 GPIO_ACTIVE_HIGH>,
>> + <&main_gpio1 9 GPIO_ACTIVE_HIGH>,
>> + <&main_gpio1 24 GPIO_ACTIVE_HIGH>,
>> + <&main_gpio1 25 GPIO_ACTIVE_HIGH>,
>> + <&main_gpio1 22 GPIO_ACTIVE_HIGH>,
>> + <&main_gpio1 23 GPIO_ACTIVE_HIGH>,
>> + <&main_gpio1 7 GPIO_ACTIVE_HIGH>,
>> + <&main_gpio1 8 GPIO_ACTIVE_HIGH>,
>> + <&main_gpio1 14 GPIO_ACTIVE_HIGH>,
>> + <&main_gpio1 13 GPIO_ACTIVE_HIGH>,
>> + <&main_gpio1 12 GPIO_ACTIVE_HIGH>,
>> + <&main_gpio1 10 GPIO_ACTIVE_HIGH>;
>> +
>> + spi-controller = <&main_spi2>;
>> + spi-cs = <0>;
>> + spi-cs-names = "default";
>> +
>> + board = <&lsm6dsl_click>;
>> + };
>> +
>> + mikrobus_boards {
>> + thermo_click: thermo-click {
>> + compatible = "maxim,max31855k", "mikrobus-spi";
>
> I might be missing something, but your solution cannot possibly be
> to list every click board that could be connected (all 1500+ of them)
> to every mikroBUS connector on every device's DT file..
I think you missed something. `mikrobus-boards` is not a child node of
`mikrobus0`. See the `board` property in `mikrobus0`. That is what
selects the board attached to the connector.
The `mikcrobus-boards` node itself should be moved to some independent
location and included from a system that wants to support mikrobus
boards. The connector will only have a phandle to the board (or boards
in case a single mikroBUS board has 1 i2c and 1 spi sensor or some
combination).
>
> Each click board should have a single DTSO overlay file to describe the
> click board, one per click board total. And then that overlay should
> apply cleanly to any device that has a mikroBUS interface.
Yes, that is the goal.
>
> Which means you have not completely solved the fundamental problem of
> abstracting the mikroBUS connector in DT. Each of these click device
> child
> nodes has to be under the parent connector node. Which means a phandle
> to the parent node, which is not generically named. For instance
> if my board has 2 connectors, I would have mikrobus0 and mikrobus1,
> the click board's overlay would look like this:
>
> /dts-v1/;
> /plugin/;
>
> &mikrobus0 {
> status = "okay";
>
> mikrobus_board {
> thermo-click {
> compatible = "maxim,max31855k", "mikrobus-spi";
> spi-max-frequency = <1000000>;
> pinctrl-apply = "spi_default";
> };
> };
> };
No, it will look as follows:
```
&mikrobus0 {
status = "okay";
board = <&thermo-click>;
};
&mikrobus_board {
thermo-click {
compatible = "maxim,max31855k", "mikrobus-spi";
spi-max-frequency = <1000000>;
pinctrl-apply = "spi_default";
};
};
```
>
> I think this solution is almost there, but once you solve the above
> issue, we could just apply the right overlay for our attached click
> board ahead of time and not need the mikroBUS bus driver at all.
Well, the driver is still needed because some things cannot be done
generically in dt. Eg:
1. SPI chipselect. Each connector will have different chipselect number
mapped to CS pin. In fact a mikrobus board might use other pins like RST
as chipselect as well.
2. Using pins other than their intended purpose like GPIO.
>
> Andrew
>
>> + spi-max-frequency = <1000000>;
>> + pinctrl-apply = "spi_default";
>> + };
>> +
>> + lsm6dsl_click: lsm6dsl-click {
>> + compatible = "st,lsm6ds3", "mikrobus-spi";
>> + spi-max-frequency = <1000000>;
>> + pinctrl-apply = "spi_default";
>> + };
>> + };
>> };
>> &main_pmx0 {
>> @@ -387,6 +438,18 @@ AM62X_IOPAD(0x01f0, PIN_OUTPUT, 5) /* (A18)
>> EXT_REFCLK1.CLKOUT0 */
>> >;
>> };
>> + mikrobus_pwm_pins_default: mikrobus-pwm-default-pins {
>> + pinctrl-single,pins = <
>> + AM62X_IOPAD(0x01a4, PIN_INPUT, 2) /* (B20)
>> MCASP0_ACLKX.ECAP2_IN_APWM_OUT */
>> + >;
>> + };
>> +
>> + mikrobus_pwm_pins_gpio: mikrobus-pwm-gpio-pins {
>> + pinctrl-single,pins = <
>> + AM62X_IOPAD(0x01a4, PIN_INPUT, 7) /* (B20)
>> MCASP0_ACLKX.GPIO1_11 */
>> + >;
>> + };
>> +
>> mikrobus_i2c_pins_default: mikrobus-i2c-default-pins {
>> pinctrl-single,pins = <
>> AM62X_IOPAD(0x01d0, PIN_INPUT_PULLUP, 2) /* (A15)
>> UART0_CTSn.I2C3_SCL */
>> @@ -394,6 +457,13 @@ AM62X_IOPAD(0x01d4, PIN_INPUT_PULLUP, 2) /*
>> (B15) UART0_RTSn.I2C3_SDA */
>> >;
>> };
>> + mikrobus_i2c_pins_gpio: mikrobus-i2c-gpio-pins {
>> + pinctrl-single,pins = <
>> + AM62X_IOPAD(0x01d0, PIN_INPUT, 7) /* (A15)
>> UART0_CTSn.GPIO1_22 */
>> + AM62X_IOPAD(0x01d4, PIN_INPUT, 7) /* (B15)
>> UART0_RTSn.GPIO1_23 */
>> + >;
>> + };
>> +
>> mikrobus_uart_pins_default: mikrobus-uart-default-pins {
>> pinctrl-single,pins = <
>> AM62X_IOPAD(0x01d8, PIN_INPUT, 1) /* (C15)
>> MCAN0_TX.UART5_RXD */
>> @@ -401,6 +471,13 @@ AM62X_IOPAD(0x01dc, PIN_OUTPUT, 1) /* (E15)
>> MCAN0_RX.UART5_TXD */
>> >;
>> };
>> + mikrobus_uart_pins_gpio: mikrobus-uart-gpio-pins {
>> + pinctrl-single,pins = <
>> + AM62X_IOPAD(0x01d8, PIN_INPUT, 7) /* (C15)
>> MCAN0_TX.GPIO1_24 */
>> + AM62X_IOPAD(0x01dc, PIN_INPUT, 7) /* (E15)
>> MCAN0_RX.GPIO1_25 */
>> + >;
>> + };
>> +
>> mikrobus_spi_pins_default: mikrobus-spi-default-pins {
>> pinctrl-single,pins = <
>> AM62X_IOPAD(0x01b0, PIN_INPUT, 1) /* (A20)
>> MCASP0_ACLKR.SPI2_CLK */
>> @@ -410,6 +487,15 @@ AM62X_IOPAD(0x0198, PIN_INPUT, 1) /* (A19)
>> MCASP0_AXR2.SPI2_D1 */
>> >;
>> };
>> + mikrobus_spi_pins_gpio: mikrobus-spi-gpio-pins {
>> + pinctrl-single,pins = <
>> + AM62X_IOPAD(0x0194, PIN_INPUT, 7) /* (B19)
>> MCASP0_AXR3.GPIO1_7 */
>> + AM62X_IOPAD(0x0198, PIN_INPUT, 7) /* (A19)
>> MCASP0_AXR2.GPIO1_8 */
>> + AM62X_IOPAD(0x01ac, PIN_INPUT, 7) /* (E19)
>> MCASP0_AFSR.GPIO1_13 */
>> + AM62X_IOPAD(0x01b0, PIN_INPUT, 7) /* (A20)
>> MCASP0_ACLKR.GPIO1_14 */
>> + >;
>> + };
>> +
>> mikrobus_gpio_pins_default: mikrobus-gpio-default-pins {
>> bootph-all;
>> pinctrl-single,pins = <
>> @@ -630,8 +716,6 @@ &main_gpio0 {
>> &main_gpio1 {
>> bootph-all;
>> - pinctrl-names = "default";
>> - pinctrl-0 = <&mikrobus_gpio_pins_default>;
>> gpio-line-names = "", "", "", "", "", /* 0-4 */
>> "SPE_RSTN", "SPE_INTN", "MIKROBUS_GPIO1_7", /* 5-7 */
>> "MIKROBUS_GPIO1_8", "MIKROBUS_GPIO1_9", /* 8-9 */
>> @@ -804,15 +888,11 @@ it66121_out: endpoint {
>> };
>> &main_i2c3 {
>> - pinctrl-names = "default";
>> - pinctrl-0 = <&mikrobus_i2c_pins_default>;
>> clock-frequency = <400000>;
>> status = "okay";
>> };
>> &main_spi2 {
>> - pinctrl-names = "default";
>> - pinctrl-0 = <&mikrobus_spi_pins_default>;
>> status = "okay";
>> };
>> @@ -876,8 +956,6 @@ &main_uart1 {
>> };
>> &main_uart5 {
>> - pinctrl-names = "default";
>> - pinctrl-0 = <&mikrobus_uart_pins_default>;
>> status = "okay";
>> };
>>
Ayush Singh
Powered by blists - more mailing lists