From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 53635C3064D for ; Thu, 27 Jun 2024 17:16:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=z7foaEoqWBYAVfICkiWLM0lOscLmLoi1AUMgb9qBaJs=; b=c1zhQu8pZ6OnkSwXBMfoxYQaJg Gpc5c14abrZvazLuoalSV/mvq0bqCVGQFAZGtxFh1HjK9T9w+L1dgt9hvRepQUZy42azsoQwD6E01 GJ99YD3jwCy/JcmwVzYtb8Y39pedxFZz0xNP29HeiQKwtLxog9sTJyNxiUgW0o7gm4IyRYF5mytyb mipH3Ygks7FWU6YBjbqOV37qCRFkN3jDkB6vqtbk1meQkTrmYz9LPnh6dfmkQUXs0OUmN7UH1QXq5 76JJV6QvNv9X39xr3YtduTfu6VmkT5rs4ib1AyFBhBcJK/pUDMupYMp34LDb3cxMvRNWFsaUc5YVf r92F4XFQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sMsjb-0000000BAVY-0Sgo; Thu, 27 Jun 2024 17:16:35 +0000 Received: from mail-pl1-x635.google.com ([2607:f8b0:4864:20::635]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sMsjR-0000000BATA-2Kpr for linux-arm-kernel@lists.infradead.org; Thu, 27 Jun 2024 17:16:27 +0000 Received: by mail-pl1-x635.google.com with SMTP id d9443c01a7336-1fa42ad2701so5801525ad.1 for ; Thu, 27 Jun 2024 10:16:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=beagleboard-org.20230601.gappssmtp.com; s=20230601; t=1719508585; x=1720113385; darn=lists.infradead.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=z7foaEoqWBYAVfICkiWLM0lOscLmLoi1AUMgb9qBaJs=; b=INUHrpwGJCc/VFwM2p4okUqfz2w38zk32UzZn+bFsYb94OzpWrDmzU5msS5uHrK4e/ OnqIhfPI9fJ3sodI2kqE8bgsPTscWmNLqXxB0RpVCGLinRUinRYjefNscGuzT2J53Qxi vhK3VPrg5lHjq0IyOwVWbS1Dpr3NYiKJZp2sUeRVLdOZEvMxAgxW3J+2P0X6fjRGS1r+ +Gl9Kx3vO0YeVhGIJj2szXFVbuIHKxi/bZ+0KB5RJHiRVbakOfMDWWS9QAfbR6/FPt4X RwvKExuJmENqPrxldYnF5Y6VbhuqW3ncQXZnddpeQP4wNx+ASGWWXYdComXQM3zmzxIl kHNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719508585; x=1720113385; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=z7foaEoqWBYAVfICkiWLM0lOscLmLoi1AUMgb9qBaJs=; b=i1aQLsumMv/AIz8/OBQxTOWTqFkpTGpz0TuF2XpQ1Wp0Q+/oGzCj2k0eIpcPBXIyTb GWY948FTKLSV827011Pia8RojO7nBSvJgO4FvSPFkkQJ731hS1+RCxB4zhDoO8sQhLw0 iElLiVDc/3W0Z1dMSgrw5DB9lJHrrzbaDMufRJT7LaU1ywcTVl82pt0943pKLRlkYPmG oQEmk45psaUH+7DRtrTNU9itAK+R5s4lPtqrQRvZdla78ZiUuUxNQDRzIsIJnDXs9K3C btjEFYp1xZJGHbNJZkyGvS4J/WwfqBCZYloCI6sVezUKuAaMGjmzviKAmtZ6mz3h8GgP 42dw== X-Forwarded-Encrypted: i=1; AJvYcCUChl5OEf6x/gGuRmpQ++uJlU8TwnuSpTbmdfge9BRdewPk7WnNaAyQxHFqdyXKszRc+tLWO6Mezfg3KfEApqSat2+UwHf5uF1SkRJHHfN4XgXUOes= X-Gm-Message-State: AOJu0YyAQ9ImFjVseLxpw72Gkfp8yco+Zm0lRGeYVrn6e3Ws5FZG7Hnx v6nbSKF1DX2wXhCDVskR2NNNcM4SJT9bUj9OhWzehpJcM3zA+UD3S2hvEGv2K040Cq+LXD3FORd 37Q== X-Google-Smtp-Source: AGHT+IHocJw3g7IpJwxKwOC/B8eIwHeYixMRJQpnA/X78oiJ15427+UGqnMhiNsCyVfkiUfhChbEhQ== X-Received: by 2002:a05:6a20:9794:b0:1bc:ecc1:6f7f with SMTP id adf61e73a8af0-1bcecc1709emr14438688637.1.1719508584653; Thu, 27 Jun 2024 10:16:24 -0700 (PDT) Received: from ?IPV6:2401:4900:1f3e:18b0:f314:9f76:9f94:eb43? ([2401:4900:1f3e:18b0:f314:9f76:9f94:eb43]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-706b4a35418sm1656485b3a.177.2024.06.27.10.16.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 27 Jun 2024 10:16:24 -0700 (PDT) Message-ID: Date: Thu, 27 Jun 2024 22:46:15 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 7/7] dts: ti: k3-am625-beagleplay: Add mikroBUS Content-Language: en-US To: Andrew Davis , Mark Brown , Vaishnav M A , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Derek Kiernan , Dragan Cvetic , Arnd Bergmann , Greg Kroah-Hartman , Nishanth Menon , Vignesh Raghavendra , Tero Kristo , Michael Walle , Andrew Lunn , jkridner@beagleboard.org, robertcnelson@beagleboard.org Cc: linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org References: <20240627-mikrobus-scratch-spi-v5-0-9e6c148bf5f0@beagleboard.org> <20240627-mikrobus-scratch-spi-v5-7-9e6c148bf5f0@beagleboard.org> <4e23ec81-b278-4f2b-815d-64ed9390ca55@ti.com> From: Ayush Singh In-Reply-To: <4e23ec81-b278-4f2b-815d-64ed9390ca55@ti.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240627_101625_626048_ECF2E5F2 X-CRM114-Status: GOOD ( 30.90 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 >> --- >>   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