diff mbox series

[3/8] mikrobus: Add mikrobus driver

Message ID 20240911-mikrobus-dt-v1-3-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
A simple platform driver for now that does nothing. This is required
because without a platform driver, the mikrobus_gpio0 nexus node cannot
be used.

In future, this driver will also allow for dynamic board detection using
1-wire eeprom in new mikrobus boards.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 MAINTAINERS              |  1 +
 drivers/misc/Kconfig     | 17 +++++++++++++++++
 drivers/misc/Makefile    |  1 +
 drivers/misc/mikrobus.rs | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+)

Comments

Greg Kroah-Hartman Sept. 11, 2024, 2:57 p.m. UTC | #1
On Wed, Sep 11, 2024 at 07:57:20PM +0530, Ayush Singh wrote:
> A simple platform driver for now that does nothing. This is required
> because without a platform driver, the mikrobus_gpio0 nexus node cannot
> be used.
> 
> In future, this driver will also allow for dynamic board detection using
> 1-wire eeprom in new mikrobus boards.
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> ---
>  MAINTAINERS              |  1 +
>  drivers/misc/Kconfig     | 17 +++++++++++++++++
>  drivers/misc/Makefile    |  1 +
>  drivers/misc/mikrobus.rs | 32 ++++++++++++++++++++++++++++++++
>  4 files changed, 51 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0cc27446b18a..d0c18bd7b558 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15433,6 +15433,7 @@ MIKROBUS CONNECTOR
>  M:	Ayush Singh <ayush@beagleboard.org>
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
> +F:	drivers/misc/mikrobus.rs
>  
>  MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT
>  M:	Luka Kovacic <luka.kovacic@sartura.hr>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 3fe7e2a9bd29..30defb522e98 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -610,6 +610,23 @@ config MARVELL_CN10K_DPI
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called mrvl_cn10k_dpi.
>  
> +menuconfig MIKROBUS
> +	tristate "Module for instantiating devices on mikroBUS ports"
> +	help
> +	  This option enables the mikroBUS driver. mikroBUS is an add-on
> +	  board socket standard that offers maximum expandability with
> +	  the smallest number of pins. The mikroBUS driver instantiates
> +	  devices on a mikroBUS port described by identifying data present
> +	  in an add-on board resident EEPROM, more details on the mikroBUS
> +	  driver support and discussion can be found in this eLinux wiki :
> +	  elinux.org/Mikrobus

So you want to be a bus?  Or just a single driver?  I'm confused, what
exactly is this supposed to do?

If a bus, great, let's tie into the proper driver core bus code, don't
"open code" all of that, as that's just going to make things messier and
harder to work overall in the end.

If a single driver, why is it called "bus"?  :)

thanks,

greg k-h
Andrew Lunn Sept. 11, 2024, 3:48 p.m. UTC | #2
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -610,6 +610,23 @@ config MARVELL_CN10K_DPI
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called mrvl_cn10k_dpi.
>  
> +menuconfig MIKROBUS
> +	tristate "Module for instantiating devices on mikroBUS ports"
> +	help
> +	  This option enables the mikroBUS driver. mikroBUS is an add-on

You are missing

        depends on RUST

	Andrew
Ayush Singh Sept. 11, 2024, 4:02 p.m. UTC | #3
On 9/11/24 20:27, Greg Kroah-Hartman wrote:

> On Wed, Sep 11, 2024 at 07:57:20PM +0530, Ayush Singh wrote:
>> A simple platform driver for now that does nothing. This is required
>> because without a platform driver, the mikrobus_gpio0 nexus node cannot
>> be used.
>>
>> In future, this driver will also allow for dynamic board detection using
>> 1-wire eeprom in new mikrobus boards.
>>
>> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
>> ---
>>   MAINTAINERS              |  1 +
>>   drivers/misc/Kconfig     | 17 +++++++++++++++++
>>   drivers/misc/Makefile    |  1 +
>>   drivers/misc/mikrobus.rs | 32 ++++++++++++++++++++++++++++++++
>>   4 files changed, 51 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 0cc27446b18a..d0c18bd7b558 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -15433,6 +15433,7 @@ MIKROBUS CONNECTOR
>>   M:	Ayush Singh <ayush@beagleboard.org>
>>   S:	Maintained
>>   F:	Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
>> +F:	drivers/misc/mikrobus.rs
>>   
>>   MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT
>>   M:	Luka Kovacic <luka.kovacic@sartura.hr>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 3fe7e2a9bd29..30defb522e98 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -610,6 +610,23 @@ config MARVELL_CN10K_DPI
>>   	  To compile this driver as a module, choose M here: the module
>>   	  will be called mrvl_cn10k_dpi.
>>   
>> +menuconfig MIKROBUS
>> +	tristate "Module for instantiating devices on mikroBUS ports"
>> +	help
>> +	  This option enables the mikroBUS driver. mikroBUS is an add-on
>> +	  board socket standard that offers maximum expandability with
>> +	  the smallest number of pins. The mikroBUS driver instantiates
>> +	  devices on a mikroBUS port described by identifying data present
>> +	  in an add-on board resident EEPROM, more details on the mikroBUS
>> +	  driver support and discussion can be found in this eLinux wiki :
>> +	  elinux.org/Mikrobus
> So you want to be a bus?  Or just a single driver?  I'm confused, what
> exactly is this supposed to do?
>
> If a bus, great, let's tie into the proper driver core bus code, don't
> "open code" all of that, as that's just going to make things messier and
> harder to work overall in the end.
>
> If a single driver, why is it called "bus"?  :)
>
> thanks,
>
> greg k-h


Well, mikroBUS [0] is the name of the socket standard. It is basically a 
group of following pins:

Analog - AN
Reset - RST
SPI Chip Select - CS
SPI Clock - SCK
SPI Master Input Slave Output - MISO
SPI Master Output Slave Input - MOSI
VCC-3.3V power - +3.3V
Reference Ground - GND
PWM - PWM output
INT - Hardware Interrupt
RX - UART Receive
TX - UART Transmit
SCL - I2C Clock
SDA - I2C Data
+5V - VCC-5V power
GND - Reference Ground


I do not think it would qualify as as a "bus" in the Linux driver sense. 
Especially with the devicetree based approach here which applies overlay 
directly to the actual uart/i2c/spi controllers and basically not 
interact with the mikroBUS node much.


The driver is here to enable the following:

1. Enable dynamic board detection using 1-wire eeprom on some addon boards.

2. Provide sysfs entry for runtime board adding/removal

3. Enable using mikrobus connector node as nexus node for GPIO (not 
having a platform driver makes any driver trying to use the connector as 
nexus node go into deffered probing state).


For this patch series, the driver is mostly here due to point 3. 
Basically a stub.


[0]: https://www.mikroe.com/mikrobus

Ayush Singh
Greg Kroah-Hartman Sept. 11, 2024, 8:03 p.m. UTC | #4
On Wed, Sep 11, 2024 at 09:32:21PM +0530, Ayush Singh wrote:
> On 9/11/24 20:27, Greg Kroah-Hartman wrote:
> 
> > On Wed, Sep 11, 2024 at 07:57:20PM +0530, Ayush Singh wrote:
> > > A simple platform driver for now that does nothing. This is required
> > > because without a platform driver, the mikrobus_gpio0 nexus node cannot
> > > be used.
> > > 
> > > In future, this driver will also allow for dynamic board detection using
> > > 1-wire eeprom in new mikrobus boards.
> > > 
> > > Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> > > ---
> > >   MAINTAINERS              |  1 +
> > >   drivers/misc/Kconfig     | 17 +++++++++++++++++
> > >   drivers/misc/Makefile    |  1 +
> > >   drivers/misc/mikrobus.rs | 32 ++++++++++++++++++++++++++++++++
> > >   4 files changed, 51 insertions(+)
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 0cc27446b18a..d0c18bd7b558 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -15433,6 +15433,7 @@ MIKROBUS CONNECTOR
> > >   M:	Ayush Singh <ayush@beagleboard.org>
> > >   S:	Maintained
> > >   F:	Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
> > > +F:	drivers/misc/mikrobus.rs
> > >   MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT
> > >   M:	Luka Kovacic <luka.kovacic@sartura.hr>
> > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > > index 3fe7e2a9bd29..30defb522e98 100644
> > > --- a/drivers/misc/Kconfig
> > > +++ b/drivers/misc/Kconfig
> > > @@ -610,6 +610,23 @@ config MARVELL_CN10K_DPI
> > >   	  To compile this driver as a module, choose M here: the module
> > >   	  will be called mrvl_cn10k_dpi.
> > > +menuconfig MIKROBUS
> > > +	tristate "Module for instantiating devices on mikroBUS ports"
> > > +	help
> > > +	  This option enables the mikroBUS driver. mikroBUS is an add-on
> > > +	  board socket standard that offers maximum expandability with
> > > +	  the smallest number of pins. The mikroBUS driver instantiates
> > > +	  devices on a mikroBUS port described by identifying data present
> > > +	  in an add-on board resident EEPROM, more details on the mikroBUS
> > > +	  driver support and discussion can be found in this eLinux wiki :
> > > +	  elinux.org/Mikrobus
> > So you want to be a bus?  Or just a single driver?  I'm confused, what
> > exactly is this supposed to do?
> > 
> > If a bus, great, let's tie into the proper driver core bus code, don't
> > "open code" all of that, as that's just going to make things messier and
> > harder to work overall in the end.
> > 
> > If a single driver, why is it called "bus"?  :)
> > 
> > thanks,
> > 
> > greg k-h
> 
> 
> Well, mikroBUS [0] is the name of the socket standard. It is basically a
> group of following pins:
> 
> Analog - AN
> Reset - RST
> SPI Chip Select - CS
> SPI Clock - SCK
> SPI Master Input Slave Output - MISO
> SPI Master Output Slave Input - MOSI
> VCC-3.3V power - +3.3V
> Reference Ground - GND
> PWM - PWM output
> INT - Hardware Interrupt
> RX - UART Receive
> TX - UART Transmit
> SCL - I2C Clock
> SDA - I2C Data
> +5V - VCC-5V power
> GND - Reference Ground
> 
> 
> I do not think it would qualify as as a "bus" in the Linux driver sense.
> Especially with the devicetree based approach here which applies overlay
> directly to the actual uart/i2c/spi controllers and basically not interact
> with the mikroBUS node much.

It will be a "bus" in the driver sense as you want to have different
drivers bound to devices that are plugged in here, right?  Or if this is
just a single driver for the hardware, then no, as you will not have any
additional drivers for this standard?  It's unclear you want to do here.

> The driver is here to enable the following:
> 
> 1. Enable dynamic board detection using 1-wire eeprom on some addon boards.

Some, not all?

> 2. Provide sysfs entry for runtime board adding/removal

That's not what sysfs is for, but we can get there eventually...

> 3. Enable using mikrobus connector node as nexus node for GPIO (not having a
> platform driver makes any driver trying to use the connector as nexus node
> go into deffered probing state).

Why is this a platform device?  Is this always going to be described by
DT somehow?

> For this patch series, the driver is mostly here due to point 3. Basically a
> stub.

Let's see how this really works before coming to conclusions.

As an example, how do you think sysfs should look for this device?  For
all devices?  Write out the needed sysfs documentation entries first and
then that will help explain things better.

thanks,

greg k-h
Ayush Singh Sept. 12, 2024, 1:32 p.m. UTC | #5
On 9/12/24 01:33, Greg Kroah-Hartman wrote:

> On Wed, Sep 11, 2024 at 09:32:21PM +0530, Ayush Singh wrote:
>> On 9/11/24 20:27, Greg Kroah-Hartman wrote:
>>
>>> On Wed, Sep 11, 2024 at 07:57:20PM +0530, Ayush Singh wrote:
>>>> A simple platform driver for now that does nothing. This is required
>>>> because without a platform driver, the mikrobus_gpio0 nexus node cannot
>>>> be used.
>>>>
>>>> In future, this driver will also allow for dynamic board detection using
>>>> 1-wire eeprom in new mikrobus boards.
>>>>
>>>> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
>>>> ---
>>>>    MAINTAINERS              |  1 +
>>>>    drivers/misc/Kconfig     | 17 +++++++++++++++++
>>>>    drivers/misc/Makefile    |  1 +
>>>>    drivers/misc/mikrobus.rs | 32 ++++++++++++++++++++++++++++++++
>>>>    4 files changed, 51 insertions(+)
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 0cc27446b18a..d0c18bd7b558 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -15433,6 +15433,7 @@ MIKROBUS CONNECTOR
>>>>    M:	Ayush Singh <ayush@beagleboard.org>
>>>>    S:	Maintained
>>>>    F:	Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
>>>> +F:	drivers/misc/mikrobus.rs
>>>>    MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT
>>>>    M:	Luka Kovacic <luka.kovacic@sartura.hr>
>>>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>>>> index 3fe7e2a9bd29..30defb522e98 100644
>>>> --- a/drivers/misc/Kconfig
>>>> +++ b/drivers/misc/Kconfig
>>>> @@ -610,6 +610,23 @@ config MARVELL_CN10K_DPI
>>>>    	  To compile this driver as a module, choose M here: the module
>>>>    	  will be called mrvl_cn10k_dpi.
>>>> +menuconfig MIKROBUS
>>>> +	tristate "Module for instantiating devices on mikroBUS ports"
>>>> +	help
>>>> +	  This option enables the mikroBUS driver. mikroBUS is an add-on
>>>> +	  board socket standard that offers maximum expandability with
>>>> +	  the smallest number of pins. The mikroBUS driver instantiates
>>>> +	  devices on a mikroBUS port described by identifying data present
>>>> +	  in an add-on board resident EEPROM, more details on the mikroBUS
>>>> +	  driver support and discussion can be found in this eLinux wiki :
>>>> +	  elinux.org/Mikrobus
>>> So you want to be a bus?  Or just a single driver?  I'm confused, what
>>> exactly is this supposed to do?
>>>
>>> If a bus, great, let's tie into the proper driver core bus code, don't
>>> "open code" all of that, as that's just going to make things messier and
>>> harder to work overall in the end.
>>>
>>> If a single driver, why is it called "bus"?  :)
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>> Well, mikroBUS [0] is the name of the socket standard. It is basically a
>> group of following pins:
>>
>> Analog - AN
>> Reset - RST
>> SPI Chip Select - CS
>> SPI Clock - SCK
>> SPI Master Input Slave Output - MISO
>> SPI Master Output Slave Input - MOSI
>> VCC-3.3V power - +3.3V
>> Reference Ground - GND
>> PWM - PWM output
>> INT - Hardware Interrupt
>> RX - UART Receive
>> TX - UART Transmit
>> SCL - I2C Clock
>> SDA - I2C Data
>> +5V - VCC-5V power
>> GND - Reference Ground
>>
>>
>> I do not think it would qualify as as a "bus" in the Linux driver sense.
>> Especially with the devicetree based approach here which applies overlay
>> directly to the actual uart/i2c/spi controllers and basically not interact
>> with the mikroBUS node much.
> It will be a "bus" in the driver sense as you want to have different
> drivers bound to devices that are plugged in here, right?  Or if this is
> just a single driver for the hardware, then no, as you will not have any
> additional drivers for this standard?  It's unclear you want to do here.
No, a single driver. The driver is for board detection and applying 
proper overlay. There will not be separate drivers for each addon board.

>> The driver is here to enable the following:
>>
>> 1. Enable dynamic board detection using 1-wire eeprom on some addon boards.
> Some, not all?

Some mikroBUS boards have 1-wire eeprom [0]. This can be used to enable 
dynamic detection of the board. But this is not part of mikroBUS 
specification itself, and is like a mikroe addition. So there will be a 
lot of mikrobus addon boards that do not have any way to do dynamic 
detection.

>> 2. Provide sysfs entry for runtime board adding/removal
> That's not what sysfs is for, but we can get there eventually...

>> 3. Enable using mikrobus connector node as nexus node for GPIO (not having a
>> platform driver makes any driver trying to use the connector as nexus node
>> go into deffered probing state).
> Why is this a platform device?  Is this always going to be described by
> DT somehow?


Well, since greybus does not use a dt based approach right now, that is 
one case where we potentially cannot describe the connector in DT. But 
since greybus is mostly staging, I am not sure if there are any plans to 
use DT before bringing it to mainline.


>> For this patch series, the driver is mostly here due to point 3. Basically a
>> stub.
> Let's see how this really works before coming to conclusions.
>
> As an example, how do you think sysfs should look for this device?  For
> all devices?  Write out the needed sysfs documentation entries first and
> then that will help explain things better.
>
> thanks,
>
> greg k-h


Something like the following:

cat board-overlay.dtbo > /sys/bus/platform/mikrobus-connector0/new_device


And to remove the device (revert the overlay)

echo 1 > /sys/bus/platform/mikrobus-connector0/delete_device


The main motivation for the sysfs entry is that only the board overlay 
needs to be provided, since the driver already knows it's own connector 
(and hence the symbols overlay for the connector).


[0]: https://github.com/MikroElektronika/click_id


Ayush Singh
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 0cc27446b18a..d0c18bd7b558 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15433,6 +15433,7 @@  MIKROBUS CONNECTOR
 M:	Ayush Singh <ayush@beagleboard.org>
 S:	Maintained
 F:	Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
+F:	drivers/misc/mikrobus.rs
 
 MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT
 M:	Luka Kovacic <luka.kovacic@sartura.hr>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 3fe7e2a9bd29..30defb522e98 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -610,6 +610,23 @@  config MARVELL_CN10K_DPI
 	  To compile this driver as a module, choose M here: the module
 	  will be called mrvl_cn10k_dpi.
 
+menuconfig MIKROBUS
+	tristate "Module for instantiating devices on mikroBUS ports"
+	help
+	  This option enables the mikroBUS driver. mikroBUS is an add-on
+	  board socket standard that offers maximum expandability with
+	  the smallest number of pins. The mikroBUS driver instantiates
+	  devices on a mikroBUS port described by identifying data present
+	  in an add-on board resident EEPROM, more details on the mikroBUS
+	  driver support and discussion can be found in this eLinux wiki :
+	  elinux.org/Mikrobus
+
+
+	  Say Y here to enable support for this driver.
+
+	  To compile this code as a module, chose M here: the module
+	  will be called mikrobus.ko
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index a9f94525e181..86ea188e3cf9 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -72,3 +72,4 @@  obj-$(CONFIG_TPS6594_PFSM)	+= tps6594-pfsm.o
 obj-$(CONFIG_NSM)		+= nsm.o
 obj-$(CONFIG_MARVELL_CN10K_DPI)	+= mrvl_cn10k_dpi.o
 obj-y				+= keba/
+obj-$(CONFIG_MIKROBUS)		+= mikrobus.o
diff --git a/drivers/misc/mikrobus.rs b/drivers/misc/mikrobus.rs
new file mode 100644
index 000000000000..a52268efd71b
--- /dev/null
+++ b/drivers/misc/mikrobus.rs
@@ -0,0 +1,32 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+//! MikroBUS driver
+
+use kernel::c_str;
+use kernel::platform::{self, DeviceId};
+use kernel::prelude::*;
+
+kernel::module_platform_driver! {
+    driver: MikrobusDriver,
+    of_table: [DeviceId::new(c_str!("mikrobus-connector"))],
+    name: "mikrobus",
+    author: "Ayush Singh <ayush@beagleboard.org>",
+    description: "MikroBUS connector Driver",
+    license: "GPL",
+}
+
+struct MikrobusDriver;
+
+#[vtable]
+impl platform::Driver for MikrobusDriver {
+    const NAME: &'static CStr = c_str!("MikroBUS");
+
+    fn probe(_dev: &mut platform::Device) -> Result {
+        pr_debug!("Mikrobus Driver (probe)\n");
+        Ok(())
+    }
+
+    fn remove(_dev: &mut platform::Device) {
+        pr_debug!("Mikrobus Driver (remove)\n");
+    }
+}