diff mbox series

[8/8] addon_boards: mikrobus: Add GPS3 Click

Message ID 20240911-mikrobus-dt-v1-8-3ded4dc879e7@beagleboard.org (mailing list archive)
State New, archived
Headers show
Series Add generic overlay for MikroBUS addon boards | expand

Commit Message

Ayush Singh Sept. 11, 2024, 2:27 p.m. UTC
- GPS3 Click is a UART MikroBUS addon Board

Link: https://www.mikroe.com/gps-3-click

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 addon_boards/mikrobus/Makefile         |  1 +
 addon_boards/mikrobus/mikroe-1714.dtso | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

Comments

Greg KH Sept. 11, 2024, 2:58 p.m. UTC | #1
On Wed, Sep 11, 2024 at 07:57:25PM +0530, Ayush Singh wrote:
> - GPS3 Click is a UART MikroBUS addon Board
> 
> Link: https://www.mikroe.com/gps-3-click
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> ---
>  addon_boards/mikrobus/Makefile         |  1 +
>  addon_boards/mikrobus/mikroe-1714.dtso | 28 ++++++++++++++++++++++++++++

Odd top-level directory for the kernel, are you sure this is correct?

thanks,

greg k-h
Ayush Singh Sept. 11, 2024, 3:56 p.m. UTC | #2
On 9/11/24 20:28, Greg Kroah-Hartman wrote:

> On Wed, Sep 11, 2024 at 07:57:25PM +0530, Ayush Singh wrote:
>> - GPS3 Click is a UART MikroBUS addon Board
>>
>> Link: https://www.mikroe.com/gps-3-click
>>
>> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
>> ---
>>   addon_boards/mikrobus/Makefile         |  1 +
>>   addon_boards/mikrobus/mikroe-1714.dtso | 28 ++++++++++++++++++++++++++++
> Odd top-level directory for the kernel, are you sure this is correct?
>
> thanks,
>
> greg k-h
>
Well, it is kinda a temporary location, since well, I could not find a 
good place for board overlays but a top-level location seems better than 
putting them in any arch specific location. I am open to moving them to 
a more suitable location if we have one.


Ayush Singh
Greg KH Sept. 11, 2024, 8:04 p.m. UTC | #3
On Wed, Sep 11, 2024 at 09:26:06PM +0530, Ayush Singh wrote:
> On 9/11/24 20:28, Greg Kroah-Hartman wrote:
> 
> > On Wed, Sep 11, 2024 at 07:57:25PM +0530, Ayush Singh wrote:
> > > - GPS3 Click is a UART MikroBUS addon Board
> > > 
> > > Link: https://www.mikroe.com/gps-3-click
> > > 
> > > Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> > > ---
> > >   addon_boards/mikrobus/Makefile         |  1 +
> > >   addon_boards/mikrobus/mikroe-1714.dtso | 28 ++++++++++++++++++++++++++++
> > Odd top-level directory for the kernel, are you sure this is correct?
> > 
> > thanks,
> > 
> > greg k-h
> > 
> Well, it is kinda a temporary location, since well, I could not find a good
> place for board overlays but a top-level location seems better than putting
> them in any arch specific location. I am open to moving them to a more
> suitable location if we have one.

top-level location is not ok for something so tiny and "special".  Just
put it where all other dtso files go.

thanks,

greg k-h
Ayush Singh Sept. 12, 2024, 7:16 a.m. UTC | #4
On 9/12/24 01:34, Greg Kroah-Hartman wrote:

> On Wed, Sep 11, 2024 at 09:26:06PM +0530, Ayush Singh wrote:
>> On 9/11/24 20:28, Greg Kroah-Hartman wrote:
>>
>>> On Wed, Sep 11, 2024 at 07:57:25PM +0530, Ayush Singh wrote:
>>>> - GPS3 Click is a UART MikroBUS addon Board
>>>>
>>>> Link: https://www.mikroe.com/gps-3-click
>>>>
>>>> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
>>>> ---
>>>>    addon_boards/mikrobus/Makefile         |  1 +
>>>>    addon_boards/mikrobus/mikroe-1714.dtso | 28 ++++++++++++++++++++++++++++
>>> Odd top-level directory for the kernel, are you sure this is correct?
>>>
>>> thanks,
>>>
>>> greg k-h
>>>
>> Well, it is kinda a temporary location, since well, I could not find a good
>> place for board overlays but a top-level location seems better than putting
>> them in any arch specific location. I am open to moving them to a more
>> suitable location if we have one.
> top-level location is not ok for something so tiny and "special".  Just
> put it where all other dtso files go.
>
> thanks,
>
> greg k-h


So here are the directories where dtso files currently go:

❯ find . -type f -name "*.dtso" -printf "%h\n" | sort -u
./arch/arm64/boot/dts/amlogic
./arch/arm64/boot/dts/freescale
./arch/arm64/boot/dts/mediatek
./arch/arm64/boot/dts/qcom
./arch/arm64/boot/dts/renesas
./arch/arm64/boot/dts/rockchip
./arch/arm64/boot/dts/ti
./arch/arm64/boot/dts/xilinx
./arch/arm/boot/dts/nxp/imx
./arch/arm/boot/dts/ti/omap
./drivers/clk
./drivers/of
./drivers/of/unittest-data


Out of these, `drivers/of` and `drivers/of/unittest-data` contain 
unittest dtso, so probably not the place.

And the `arch/arm` and `arch/arm64` are for arch specific stuff. 
MikroBUS is supported in RISC-V boards as well (BeagleV-Ahead). So 
probably not the correct location either.

Maybe something like `arch/common/addon_boards` would be better?


Ayush Singh
Dirk Behme Sept. 12, 2024, 7:29 a.m. UTC | #5
On 12.09.2024 09:16, Ayush Singh wrote:
> On 9/12/24 01:34, Greg Kroah-Hartman wrote:
> 
>> On Wed, Sep 11, 2024 at 09:26:06PM +0530, Ayush Singh wrote:
>>> On 9/11/24 20:28, Greg Kroah-Hartman wrote:
>>>
>>>> On Wed, Sep 11, 2024 at 07:57:25PM +0530, Ayush Singh wrote:
>>>>> - GPS3 Click is a UART MikroBUS addon Board
>>>>>
>>>>> Link: https://www.mikroe.com/gps-3-click
>>>>>
>>>>> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
>>>>> ---
>>>>>    addon_boards/mikrobus/Makefile         |  1 +
>>>>>    addon_boards/mikrobus/mikroe-1714.dtso | 28 
>>>>> ++++++++++++++++++++++++++++
>>>> Odd top-level directory for the kernel, are you sure this is correct?
>>>>
>>>> thanks,
>>>>
>>>> greg k-h
>>>>
>>> Well, it is kinda a temporary location, since well, I could not find 
>>> a good
>>> place for board overlays but a top-level location seems better than 
>>> putting
>>> them in any arch specific location. I am open to moving them to a more
>>> suitable location if we have one.
>> top-level location is not ok for something so tiny and "special".  Just
>> put it where all other dtso files go.
>>
>> thanks,
>>
>> greg k-h
> 
> 
> So here are the directories where dtso files currently go:
> 
> ❯ find . -type f -name "*.dtso" -printf "%h\n" | sort -u
> ./arch/arm64/boot/dts/amlogic
> ./arch/arm64/boot/dts/freescale
> ./arch/arm64/boot/dts/mediatek
> ./arch/arm64/boot/dts/qcom
> ./arch/arm64/boot/dts/renesas
> ./arch/arm64/boot/dts/rockchip
> ./arch/arm64/boot/dts/ti
> ./arch/arm64/boot/dts/xilinx
> ./arch/arm/boot/dts/nxp/imx
> ./arch/arm/boot/dts/ti/omap
> ./drivers/clk
> ./drivers/of
> ./drivers/of/unittest-data
> 
> 
> Out of these, `drivers/of` and `drivers/of/unittest-data` contain 
> unittest dtso, so probably not the place.
> 
> And the `arch/arm` and `arch/arm64` are for arch specific stuff. 
> MikroBUS is supported in RISC-V boards as well (BeagleV-Ahead). So 
> probably not the correct location either.
> 
> Maybe something like `arch/common/addon_boards` would be better?

Whats about

drivers/misc/mikrobus/mikrobus.rs
drivers/misc/mikrobus/mikroe-1714.dtso
drivers/misc/mikrobus/mikroe-5761-i2c.dtso

?

Dirk
Greg KH Sept. 12, 2024, 7:39 a.m. UTC | #6
On Thu, Sep 12, 2024 at 09:29:01AM +0200, Dirk Behme wrote:
> On 12.09.2024 09:16, Ayush Singh wrote:
> > On 9/12/24 01:34, Greg Kroah-Hartman wrote:
> > 
> > > On Wed, Sep 11, 2024 at 09:26:06PM +0530, Ayush Singh wrote:
> > > > On 9/11/24 20:28, Greg Kroah-Hartman wrote:
> > > > 
> > > > > On Wed, Sep 11, 2024 at 07:57:25PM +0530, Ayush Singh wrote:
> > > > > > - GPS3 Click is a UART MikroBUS addon Board
> > > > > > 
> > > > > > Link: https://www.mikroe.com/gps-3-click
> > > > > > 
> > > > > > Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> > > > > > ---
> > > > > >    addon_boards/mikrobus/Makefile         |  1 +
> > > > > >    addon_boards/mikrobus/mikroe-1714.dtso | 28
> > > > > > ++++++++++++++++++++++++++++
> > > > > Odd top-level directory for the kernel, are you sure this is correct?
> > > > > 
> > > > > thanks,
> > > > > 
> > > > > greg k-h
> > > > > 
> > > > Well, it is kinda a temporary location, since well, I could not
> > > > find a good
> > > > place for board overlays but a top-level location seems better
> > > > than putting
> > > > them in any arch specific location. I am open to moving them to a more
> > > > suitable location if we have one.
> > > top-level location is not ok for something so tiny and "special".  Just
> > > put it where all other dtso files go.
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > 
> > So here are the directories where dtso files currently go:
> > 
> > ❯ find . -type f -name "*.dtso" -printf "%h\n" | sort -u
> > ./arch/arm64/boot/dts/amlogic
> > ./arch/arm64/boot/dts/freescale
> > ./arch/arm64/boot/dts/mediatek
> > ./arch/arm64/boot/dts/qcom
> > ./arch/arm64/boot/dts/renesas
> > ./arch/arm64/boot/dts/rockchip
> > ./arch/arm64/boot/dts/ti
> > ./arch/arm64/boot/dts/xilinx
> > ./arch/arm/boot/dts/nxp/imx
> > ./arch/arm/boot/dts/ti/omap
> > ./drivers/clk
> > ./drivers/of
> > ./drivers/of/unittest-data
> > 
> > 
> > Out of these, `drivers/of` and `drivers/of/unittest-data` contain
> > unittest dtso, so probably not the place.
> > 
> > And the `arch/arm` and `arch/arm64` are for arch specific stuff.
> > MikroBUS is supported in RISC-V boards as well (BeagleV-Ahead). So
> > probably not the correct location either.
> > 
> > Maybe something like `arch/common/addon_boards` would be better?
> 
> Whats about
> 
> drivers/misc/mikrobus/mikrobus.rs
> drivers/misc/mikrobus/mikroe-1714.dtso
> drivers/misc/mikrobus/mikroe-5761-i2c.dtso

Exactly, put them where the drivers are, like clk and of does.

thanks,

greg k-h
Ayush Singh Sept. 12, 2024, 8:17 a.m. UTC | #7
On 9/12/24 13:09, Greg Kroah-Hartman wrote:
> On Thu, Sep 12, 2024 at 09:29:01AM +0200, Dirk Behme wrote:
>> On 12.09.2024 09:16, Ayush Singh wrote:
>>> On 9/12/24 01:34, Greg Kroah-Hartman wrote:
>>>
>>>> On Wed, Sep 11, 2024 at 09:26:06PM +0530, Ayush Singh wrote:
>>>>> On 9/11/24 20:28, Greg Kroah-Hartman wrote:
>>>>>
>>>>>> On Wed, Sep 11, 2024 at 07:57:25PM +0530, Ayush Singh wrote:
>>>>>>> - GPS3 Click is a UART MikroBUS addon Board
>>>>>>>
>>>>>>> Link: https://www.mikroe.com/gps-3-click
>>>>>>>
>>>>>>> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
>>>>>>> ---
>>>>>>>     addon_boards/mikrobus/Makefile         |  1 +
>>>>>>>     addon_boards/mikrobus/mikroe-1714.dtso | 28
>>>>>>> ++++++++++++++++++++++++++++
>>>>>> Odd top-level directory for the kernel, are you sure this is correct?
>>>>>>
>>>>>> thanks,
>>>>>>
>>>>>> greg k-h
>>>>>>
>>>>> Well, it is kinda a temporary location, since well, I could not
>>>>> find a good
>>>>> place for board overlays but a top-level location seems better
>>>>> than putting
>>>>> them in any arch specific location. I am open to moving them to a more
>>>>> suitable location if we have one.
>>>> top-level location is not ok for something so tiny and "special".  Just
>>>> put it where all other dtso files go.
>>>>
>>>> thanks,
>>>>
>>>> greg k-h
>>>
>>> So here are the directories where dtso files currently go:
>>>
>>> ❯ find . -type f -name "*.dtso" -printf "%h\n" | sort -u
>>> ./arch/arm64/boot/dts/amlogic
>>> ./arch/arm64/boot/dts/freescale
>>> ./arch/arm64/boot/dts/mediatek
>>> ./arch/arm64/boot/dts/qcom
>>> ./arch/arm64/boot/dts/renesas
>>> ./arch/arm64/boot/dts/rockchip
>>> ./arch/arm64/boot/dts/ti
>>> ./arch/arm64/boot/dts/xilinx
>>> ./arch/arm/boot/dts/nxp/imx
>>> ./arch/arm/boot/dts/ti/omap
>>> ./drivers/clk
>>> ./drivers/of
>>> ./drivers/of/unittest-data
>>>
>>>
>>> Out of these, `drivers/of` and `drivers/of/unittest-data` contain
>>> unittest dtso, so probably not the place.
>>>
>>> And the `arch/arm` and `arch/arm64` are for arch specific stuff.
>>> MikroBUS is supported in RISC-V boards as well (BeagleV-Ahead). So
>>> probably not the correct location either.
>>>
>>> Maybe something like `arch/common/addon_boards` would be better?
>> Whats about
>>
>> drivers/misc/mikrobus/mikrobus.rs
>> drivers/misc/mikrobus/mikroe-1714.dtso
>> drivers/misc/mikrobus/mikroe-5761-i2c.dtso
> Exactly, put them where the drivers are, like clk and of does.
>
> thanks,
>
> greg k-h


So I am writing a more thorough reply in the driver questions, but 
essentially, the driver is not actually required for using the overlay 
based approach for mikroBUS addon boards. Initially, the driver was not 
supposed to be included in the patch series at all. But I was not able 
to find a way to use a GPIO nexus node [0] without having a platform 
driver attached to the node.

In fact, if the GPIO nexus node is not required (like in the case of 
weather click), there is no need to even have a mikrobus-connector node 
in dt, let alone a driver.


So to answer why it probably should not go in the driver directory, the 
driver for the connector, actually does not register the mikrobus addon 
board. And if there is a way to have GPIO nexus node without having a 
platform driver attached to the node, then it should probably be removed.


The reason why the overlay based approach was suggested was because 
previous approaches could not do board stacking (having chain of 
mikrobus connector -> grove connector addon board -> grove board). So as 
you can see, it is beneficial to have grove board overlays compiled even 
in a board without any grove connectors because of stacking.


[0]: 
https://devicetree-specification.readthedocs.io/en/v0.3/devicetree-basics.html#nexus-nodes-and-specifier-mapping


Ayush Singh
Geert Stappers Sept. 12, 2024, 8:50 a.m. UTC | #8
On Thu, Sep 12, 2024 at 01:47:18PM +0530, Ayush Singh wrote:
> On 9/12/24 13:09, Greg Kroah-Hartman wrote:
> > On Thu, Sep 12, 2024 at 09:29:01AM +0200, Dirk Behme wrote:
> > > On 12.09.2024 09:16, Ayush Singh wrote:
> > > > On 9/12/24 01:34, Greg Kroah-Hartman wrote:
> > > > > On Wed, Sep 11, 2024 at 09:26:06PM +0530, Ayush Singh wrote:
> > > > > > On 9/11/24 20:28, Greg Kroah-Hartman wrote:
> > > > > > > >     addon_boards/mikrobus/Makefile         |  1 +
> > > > > > > >     addon_boards/mikrobus/mikroe-1714.dtso | 28
> > > > > > > > ++++++++++++++++++++++++++++
> > > > > > > Odd top-level directory for the kernel, are you sure this is correct?
> > > > > > I am open to moving them to a more suitable location if we have one.
> > > > 
> > > > So here are the directories where dtso files currently go:
> > > > ❯ find . -type f -name "*.dtso" -printf "%h\n" | sort -u
> > > > 
> > > > 
> > > > Out of these, `drivers/of` and `drivers/of/unittest-data` contain
> > > > unittest dtso, so probably not the place.
> > > > 
> > > > And the `arch/arm` and `arch/arm64` are for arch specific stuff.
> > > > MikroBUS is supported in RISC-V boards as well (BeagleV-Ahead). So
> > > > probably not the correct location either.
> > > > 
> > > > Maybe something like `arch/common/addon_boards` would be better?
> > > Whats about
> > > 
> > > drivers/misc/mikrobus/mikrobus.rs
> > > drivers/misc/mikrobus/mikroe-1714.dtso
> > > drivers/misc/mikrobus/mikroe-5761-i2c.dtso
> > 
> > Exactly, put them where the drivers are, like clk and of does.
> > 
> > thanks,
> > greg k-h
> 
> 
> So I am writing a more thorough reply in the driver questions,

As I did read it,
is it not "driver questions" but about "location of new files"
and "bus versus device"


> but essentially, the driver is not actually required for using the
> overlay based approach for mikroBUS addon boards. Initially, the driver
> was not supposed to be included in the patch series at all. But I was
> not able to find a way to use a GPIO nexus node [0] without having a
> platform driver attached to the node.
> 
> In fact, if the GPIO nexus node is not required (like in the case of
> weather click), there is no need to even have a mikrobus-connector
> node in dt, let alone a driver.
> 
> So to answer why it probably should not go in the driver directory,
> the driver for the connector, actually does not register the mikrobus
> addon board. And if there is a way to have GPIO nexus node without
> having a platform driver attached to the node, then it should probably
> be removed.
> 
> The reason why the overlay based approach was suggested was because
> previous approaches could not do board stacking (having chain of
> mikrobus connector -> grove connector addon board -> grove board). So
> as you can see, it is beneficial to have grove board overlays compiled
> even in a board without any grove connectors because of stacking.

Please be explicite about file location.

And please elaborate on "bus" in mikrobus.


Make it possible that your audience gets a completere picture.


And for plan B: How important is this patch to the patch serie?


Groeten
Geert Stappers

[0]: https://devicetree-specification.readthedocs.io/en/v0.3/devicetree-basics.html#nexus-nodes-and-specifier-mapping
diff mbox series

Patch

diff --git a/addon_boards/mikrobus/Makefile b/addon_boards/mikrobus/Makefile
index 4c7a68ea9504..e4b3d1aa7001 100644
--- a/addon_boards/mikrobus/Makefile
+++ b/addon_boards/mikrobus/Makefile
@@ -1,3 +1,4 @@ 
 # SPDX-License-Identifier: GPL-2.0
 
 dtb-y += mikroe-5761-i2c.dtbo
+dtb-y += mikroe-1714.dtbo
diff --git a/addon_boards/mikrobus/mikroe-1714.dtso b/addon_boards/mikrobus/mikroe-1714.dtso
new file mode 100644
index 000000000000..c8a0872ba954
--- /dev/null
+++ b/addon_boards/mikrobus/mikroe-1714.dtso
@@ -0,0 +1,28 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/**
+ * MikroBUS - Weather Click
+ *
+ * https://www.mikroe.com/weather-click
+ *
+ * Copyright (C) 2024 Ayush Singh <ayush@beagleboard.org>
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+
+&MIKROBUS_CONNECTOR {
+	status = "okay";
+};
+
+&MIKROBUS_TX_UART {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&MIKROBUS_TX_MUX_UART_TX>;
+
+	gnss {
+		compatible = "u-blox,neo-8";
+		reset-gpios = <&MIKROBUS_ALL_GPIO 10 GPIO_ACTIVE_LOW>;
+	};
+};