rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Add generic overlay for MikroBUS addon boards
@ 2024-09-11 14:27 Ayush Singh
  2024-09-11 14:27 ` [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions Ayush Singh
                   ` (9 more replies)
  0 siblings, 10 replies; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 14:27 UTC (permalink / raw)
  To: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	Nishanth Menon, Vignesh Raghavendra, Tero Kristo
  Cc: linux-kernel, rust-for-linux, devicetree, linux-arm-kernel,
	Ayush Singh

Hello all,

This is an attempt to add MikroBUS addon support using the approach
described by Grove connector patch series [0].

The patch series tests out 2 addon boards + pwm and GPIO on the MikroBUS
connector. The boards used are GPS3 Click (for UART) [1] and Weather
Click (for I2C) [2]. Additionally, I have tested relative GPIO numbering
using devicetree nexus nodes [3].

The patch series does not attempt to do any dynamic discovery for 1-wire
eeprom MikroBUS addon boards, nor does it provide any sysfs entry for
board addition/removal. The connector driver might include them after
the basic support is ironed out, but the existing patches for dynamic
overlays work fine.

The patch series has been tested on BeaglePlay [4].

I will now go over individual parts of the patch series, but for a
better picture of the approach, it would be best to checkout [0] first.

MikroBUS connector driver
-------------------------

Just a stub platform driver for now. Will be extended in the future for
dynamic board discovery using 1-wire eeprom present in some MikroBUS
addon boards.

While it is a stub driver, disabling it will make the GPIO connector
nexus node unreachable (any driver attempting to use it will enter
differed probing). So it is required.

MikroBUS connector Devicetree
------------------------------

The connector devicetree defines the MikroBUS GPIO nexus node. This
allows using pin numbering relative to MikroBUS connector pins in the
addon boards overlay. Currently, the patch uses a clockwise numbering:

  0: PWM
  1: INT
  2: RX
  3: TX
  4: SCL
  5: SDA
  6: MOSI
  7: MISO
  8: SCK
  9: CS
  10: RST
  11: AN

Additionally, in case PWM pin is not using channel 0, a nexus node for pwm
should also be used and go in the connector devicetree.

MikroBUS connector symbols overlay
----------------------------------

To make an overlay generic we need a standard name scheme which we
use across base boards. For the connector pins the pinmux phandle
shall be:

<connector-name>_<pin-name>_mux_<pin-function>

For the parent provider phandle, we use a similar naming scheme:

<connector-name>_<pin-name>_<pin-function>

For GPIO controller, I am using `MIKROBUS_GPIO` name since with nexus
nodes, we do not need to define individual pin gpio controllers.

The string symbols can be replaced with phandles once [5] is accepted.
That will make connector stacking much simpler.

MikroBUS addon board overlay
----------------------------

The patch puts the addon board overlays in their own directory. I am
using the following naming scheme for MikroBUS addon boards:

<vendor>-<product_id>.dtso

Mikroe [6] lists this for all boards in their website, but I am not sure
if other vendors have a product_id.

This naming also makes future dynamic discovery easier, since click id
spec [7] contains vendor_id and product_id in the header.

Usage
-----

So what does this all look like? Let's take an example of a BeaglePlay
with one MikroBUS connectors for which we have physically attached a
Wather click module to the first connector. Doing ahead of time
command-line DT overlay application:

./fdtoverlay \
	-o output.dtb \
	-i k3-am625-beagleplay.dtb \
		k3-am625-beagleplay-mikrobus-connector0.dtbo mikroe-5761.dtbo

Open Items
----------

- SPI Support: 
  Currently SPI dt requires `reg` property to specify the
  chipselect to use. This, makes the SPI device overlay dependent on the
  SPI controller. Thus for SPI support, we need a way to allow defining
  SPI chipselect relative to MikroBUS pins, and not the actual device
  controller.

  One possible solution is to introduce something like `named-reg` and
  allow selecting the chipselect by string array. But this probably
  requires more discussion so I have left out SPI support for now.

  NOTE: pins other than the CS MikroBUS pin can be used as chipselect.
  See [8].

- Controller symbol duplication
  The current symbol scheme has multiple symbols for the same underlying
  controller (Eg: MIKROBUS_SCL_MUX_I2C_SCL and MIKROBUS_SDA_MUX_I2C_SDA)
  point to the same i2c controller.

  I think both of them will always use the same i2c controller, but
  maybe there are some exceptions? So I have left it as it is for this
  patch series. The same thing also applies to UART and SPI.

  NOTE: with the use of nexus node for GPIO, the GPIO controller symbol
  will be the same for all pins.

- Nexus node dt-bindings
  I am not quite sure how to deal with the nexus node properties
  (#gpio-cells, gpio-map, gpio-map-mask, gpio-map-pass-thru) since they
  seem to conflict with upstream gpio schema (gpio-controller is a
  dependency of #gpio-cells).

[0]: https://lore.kernel.org/linux-arm-kernel/20240702164403.29067-1-afd@ti.com/
[1]: https://www.mikroe.com/gps-3-click
[2]: https://www.mikroe.com/weather-click
[3]: https://devicetree-specification.readthedocs.io/en/v0.3/devicetree-basics.html#nexus-nodes-and-specifier-mapping
[4]: https://www.beagleboard.org/boards/beagleplay
[5]: https://lore.kernel.org/r/20240902-symbol-phandle-v1-0-683efb2a944b@beagleboard.org
[6]: https://www.mikroe.com/
[7]: https://github.com/MikroElektronika/click_id
[8]: https://www.mikroe.com/spi-extend-click

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
Ayush Singh (7):
      dt-bindings: connector: Add MikorBUS connector
      mikrobus: Add mikrobus driver
      dts: ti: k3-am625-beagleplay: Enable mikroBUS connector
      dts: ti: beagleplay: Add mikrobus connector symbols
      addon_boards: Add addon_boards plumbing
      addon_boards: mikrobus: Add Weather Click
      addon_boards: mikrobus: Add GPS3 Click

Fabien Parent (1):
      rust: kernel: Add Platform device and driver abstractions

 .../bindings/connector/mikrobus-connector.yaml     |  40 +++
 Kbuild                                             |   1 +
 Kconfig                                            |   2 +
 MAINTAINERS                                        |   8 +
 addon_boards/Kconfig                               |  16 +
 addon_boards/Makefile                              |   3 +
 addon_boards/mikrobus/Makefile                     |   4 +
 addon_boards/mikrobus/mikroe-1714.dtso             |  28 ++
 addon_boards/mikrobus/mikroe-5761-i2c.dtso         |  28 ++
 arch/arm64/boot/dts/ti/Makefile                    |   1 +
 .../k3-am625-beagleplay-mikrobus-connector0.dtso   |  49 +++
 arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts     |  53 ++-
 drivers/misc/Kconfig                               |  17 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/mikrobus.rs                           |  32 ++
 rust/bindings/bindings_helper.h                    |   1 +
 rust/kernel/lib.rs                                 |   1 +
 rust/kernel/platform.rs                            | 380 +++++++++++++++++++++
 18 files changed, 659 insertions(+), 6 deletions(-)
---
base-commit: 9aaeb87ce1e966169a57f53a02ba05b30880ffb8
change-id: 20240826-mikrobus-dt-52eaaadd0b1f

Best regards,
-- 
Ayush Singh <ayush@beagleboard.org>


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions
  2024-09-11 14:27 [PATCH 0/8] Add generic overlay for MikroBUS addon boards Ayush Singh
@ 2024-09-11 14:27 ` Ayush Singh
  2024-09-11 14:56   ` Greg Kroah-Hartman
  2024-09-11 14:27 ` [PATCH 2/8] dt-bindings: connector: Add MikorBUS connector Ayush Singh
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 14:27 UTC (permalink / raw)
  To: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	Nishanth Menon, Vignesh Raghavendra, Tero Kristo
  Cc: linux-kernel, rust-for-linux, devicetree, linux-arm-kernel,
	Ayush Singh

From: Fabien Parent <fabien.parent@linaro.org>

Ports Platform device and driver abstractions from Fabien's tree [0].

These abstractions do not depend on any generic driver registration and
id table. Instead, the minimal abstractions have been implemented
specifically for platform subsystem taking heavy inspiration from the
existing phy device and driver abstractions.

[0]: https://github.com/Fabo/linux/commits/fparent/rust-platform

Signed-off-by: Fabien Parent <fabien.parent@linaro.org>
Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/lib.rs              |   1 +
 rust/kernel/platform.rs         | 380 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 382 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index ae82e9c941af..10cbcdd74089 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -16,6 +16,7 @@
 #include <linux/jiffies.h>
 #include <linux/mdio.h>
 #include <linux/phy.h>
+#include <linux/platform_device.h>
 #include <linux/refcount.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index b5f4b3ce6b48..b3a318fde46c 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -42,6 +42,7 @@
 #[cfg(CONFIG_NET)]
 pub mod net;
 pub mod page;
+pub mod platform;
 pub mod prelude;
 pub mod print;
 pub mod rbtree;
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
new file mode 100644
index 000000000000..de28429f5551
--- /dev/null
+++ b/rust/kernel/platform.rs
@@ -0,0 +1,380 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Platform devices and drivers.
+//!
+//! Also called `platformdev`, `pdev`.
+//!
+//! C header: [`include/linux/platform_device.h`](../../../../include/linux/platform_device.h)
+
+use core::{marker::PhantomData, pin::Pin, ptr::addr_of_mut};
+
+use macros::vtable;
+
+use crate::{
+    bindings, device,
+    error::{from_result, Result},
+    str::CStr,
+    types::Opaque,
+};
+
+/// A platform device.
+///
+/// # Invariants
+///
+/// The field `ptr` is non-null and valid for the lifetime of the object.
+#[repr(transparent)]
+pub struct Device(Opaque<bindings::platform_device>);
+
+impl Device {
+    /// Creates a new [`Device`] instance from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// For the duration of `'a`,
+    /// - the pointer must point at a valid `platform_device`, and the caller
+    ///   must be in a context where all methods defined on this struct
+    ///   are safe to call.
+    unsafe fn from_raw<'a>(ptr: *mut bindings::platform_device) -> &'a mut Self {
+        // CAST: `Self` is a `repr(transparent)` wrapper around `bindings::platform_device`.
+        let ptr = ptr.cast::<Self>();
+        // SAFETY: by the function requirements the pointer is valid and we have unique access for
+        // the duration of `'a`.
+        unsafe { &mut *ptr }
+    }
+
+    /// Returns id of the platform device.
+    pub fn id(&self) -> i32 {
+        let platformdev = self.0.get();
+        // SAFETY: By the type invariants, we know that `self.ptr` is non-null and valid.
+        unsafe { (*platformdev).id }
+    }
+}
+
+impl AsRef<device::Device> for Device {
+    fn as_ref(&self) -> &device::Device {
+        let platformdev = self.0.get();
+        // SAFETY: By the type invariants, we know that `self.ptr` is non-null and valid.
+        unsafe { device::Device::as_ref(addr_of_mut!((*platformdev).dev)) }
+    }
+}
+
+/// An adapter for the registration of a Platform driver.
+struct Adapter<T: Driver> {
+    _p: PhantomData<T>,
+}
+
+impl<T: Driver> Adapter<T> {
+    /// # Safety
+    ///
+    /// `pdev` must be passed by the corresponding callback in `platform_driver`.
+    unsafe extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: This callback is called only in contexts
+            // where we can exclusively access `platform_device` because
+            // it's not published yet, so the accessors on `Device` are okay
+            // to call.
+            let dev = unsafe { Device::from_raw(pdev) };
+            T::probe(dev)?;
+            Ok(0)
+        })
+    }
+
+    /// # Safety
+    ///
+    /// `pdev` must be passed by the corresponding callback in `platform_driver`.
+    unsafe extern "C" fn remove_callback(pdev: *mut bindings::platform_device) {
+        // SAFETY: This callback is called only in contexts
+        // where we can exclusively access `platform_device` because
+        // it's not published yet, so the accessors on `Device` are okay
+        // to call.
+        let dev = unsafe { Device::from_raw(pdev) };
+        T::remove(dev);
+    }
+}
+
+/// Driver structure for a particular Platform driver.
+///
+/// Wraps the kernel's [`struct platform_driver`].
+/// This is used to register a driver for a particular PHY type with the kernel.
+///
+/// # Invariants
+///
+/// `self.0` is always in a valid state.
+///
+/// [`struct platform_driver`]: srctree/include/linux/platform.h
+#[repr(transparent)]
+pub struct DriverVTable(Opaque<bindings::platform_driver>);
+
+// SAFETY: `DriverVTable` doesn't expose any &self method to access internal data, so it's safe to
+// share `&DriverVTable` across execution context boundaries.
+unsafe impl Sync for DriverVTable {}
+
+impl DriverVTable {
+    /// Creates a [`DriverVTable`] instance from [`Driver`].
+    ///
+    /// This is used by [`module_platform_driver`] macro to create a static array of `phy_driver`.
+    ///
+    /// [`module_platform_driver`]: crate::module_platform_driver
+    pub const fn new<T: Driver, const C: usize>(match_tbl: &'static DeviceIdTable<C>) -> Self {
+        let drv = Opaque::new(bindings::platform_driver {
+            probe: if T::HAS_PROBE {
+                Some(Adapter::<T>::probe_callback)
+            } else {
+                None
+            },
+            __bindgen_anon_1: bindings::platform_driver__bindgen_ty_1 {
+                remove: if T::HAS_REMOVE {
+                    Some(Adapter::<T>::remove_callback)
+                } else {
+                    None
+                },
+            },
+            driver: create_device_driver::<T, C>(match_tbl),
+            // SAFETY: The rest is zeroed out to initialize `struct platform_driver`.
+            ..unsafe { core::mem::MaybeUninit::<bindings::platform_driver>::zeroed().assume_init() }
+        });
+
+        DriverVTable(drv)
+    }
+}
+
+const fn create_device_driver<T: Driver, const C: usize>(
+    match_tbl: &'static DeviceIdTable<C>,
+) -> bindings::device_driver {
+    bindings::device_driver {
+        name: T::NAME.as_char_ptr(),
+        of_match_table: match_tbl.get(),
+        // SAFETY: The rest is zeroed out to initialize `struct device_driver`.
+        ..unsafe { core::mem::MaybeUninit::<bindings::device_driver>::zeroed().assume_init() }
+    }
+}
+
+/// A platform driver.
+#[vtable]
+pub trait Driver {
+    /// The friendly name
+    const NAME: &'static CStr;
+
+    /// Sets up device-specific structures during discovery.
+    fn probe(_dev: &mut Device) -> Result;
+
+    /// Clean up device-specific structures during removal.
+    fn remove(_dev: &mut Device);
+}
+
+/// Registration structure for Platform driver.
+///
+/// Registers [`DriverVTable`] instance with the kernel. It will be unregistered when dropped.
+///
+/// # Invariants
+///
+/// The `driver` is currently registered to the kernel via `__platform_driver_register`.
+pub struct Registration(Pin<&'static DriverVTable>);
+
+// SAFETY: The only action allowed in a `Registration` instance is dropping it, which is safe to do
+// from any thread because `platform_drivers_unregister` can be called from any thread context.
+unsafe impl Send for Registration {}
+
+impl Registration {
+    /// Registers a Platform driver.
+    pub fn new(drv: Pin<&'static DriverVTable>, m: &'static crate::ThisModule) -> Registration {
+        unsafe {
+            bindings::__platform_driver_register(drv.0.get(), m.0);
+        }
+
+        Self(drv)
+    }
+}
+
+impl Drop for Registration {
+    fn drop(&mut self) {
+        unsafe { bindings::platform_driver_unregister(self.0 .0.get()) }
+    }
+}
+
+/// An identifier for Platform devices.
+///
+/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
+/// Platform driver.
+///
+/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
+pub struct DeviceId(&'static CStr);
+
+impl DeviceId {
+    /// A zeroed [`struct of_device_id`] used to signify end of of_device_id array.
+    ///
+    /// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
+    pub const ZERO: bindings::of_device_id = bindings::of_device_id {
+        // SAFETY: The rest is zeroed out to initialize `struct of_device_id`.
+        ..unsafe { core::mem::MaybeUninit::<bindings::of_device_id>::zeroed().assume_init() }
+    };
+
+    /// Create new instance
+    pub const fn new(s: &'static CStr) -> Self {
+        Self(s)
+    }
+
+    const fn compatible(&self) -> [i8; 128] {
+        let compatible = self.0.as_bytes_with_nul();
+        let mut comp = [0i8; 128];
+        let mut i = 0;
+
+        while i < compatible.len() {
+            comp[i] = compatible[i] as _;
+            i += 1;
+        }
+
+        comp
+    }
+
+    // macro use only
+    #[doc(hidden)]
+    pub const fn to_rawid(&self) -> bindings::of_device_id {
+        let comp = self.compatible();
+
+        bindings::of_device_id {
+            compatible: comp,
+            // SAFETY: The rest is zeroed out to initialize `struct of_device_id`.
+            ..unsafe { core::mem::MaybeUninit::<bindings::of_device_id>::zeroed().assume_init() }
+        }
+    }
+}
+
+/// An array of identifiers for platform driver
+#[repr(transparent)]
+pub struct DeviceIdTable<const C: usize>([bindings::of_device_id; C]);
+
+impl<const C: usize> DeviceIdTable<C> {
+    /// Create a new instance
+    pub const fn new(ids: [bindings::of_device_id; C]) -> Self {
+        Self(ids)
+    }
+
+    /// Returns a raw pointer to static table.
+    pub const fn get(&'static self) -> *const bindings::of_device_id {
+        self.0.as_ptr()
+    }
+}
+
+// SAFETY: `DeviceIdTable` is only used in C side behind a *const pointer, and thus remains
+// immutable and thus can be shared across execution context boundaries.
+unsafe impl<const C: usize> Sync for DeviceIdTable<C> {}
+
+/// Declares a kernel module for Platform drivers.
+///
+/// This creates a static [`struct platform_driver`] and registers it. It also creates an array of
+/// [`struct of_device_id`] for matching the driver to devicetree device.
+///
+/// [`struct platform_driver`]: srctree/include/linux/platform.h
+/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
+///
+/// # Examples
+///
+/// ```
+/// # mod module_platform_driver_sample {
+/// use kernel::c_str;
+/// use kernel::platform::{self, DeviceId};
+/// use kernel::prelude::*;
+///
+/// kernel::module_platform_driver! {
+///     driver: PlatformSimple,
+///     of_table: [DeviceId::new(c_str!("platform-simple"))],
+///     name: "rust_sample_platform",
+///     author: "Rust for Linux Contributors",
+///     description: "Rust sample Platform driver",
+///     license: "GPL",
+/// }
+///
+/// struct PlatformSimple;
+///
+/// #[vtable]
+/// impl platform::Driver for PlatformSimple {
+///     const NAME: &'static CStr = c_str!("PlatformSimple");
+/// }
+/// # }
+/// ```
+///
+/// This expands to the following code:
+///
+/// ```ignore
+/// use kernel::c_str;
+/// use kernel::platform::{self, DeviceId};
+/// use kernel::prelude::*;
+///
+///
+/// struct Module {
+///     _reg: $crate::platform::Registration,
+/// }
+///
+/// module! {
+///     type: Module,
+///     name: "rust_sample_platform",
+///     author: "Rust for Linux Contributors",
+///     description: "Rust sample Platform driver",
+///     license: "GPL",
+/// }
+///
+/// const _: () = {
+///     static OF_TABLE: $crate::platform::DeviceIdTable = $crate::platform::DeviceIdTable<2>([
+///         (DeviceId::new(c_str!("platform-simple"))).to_rawid(),
+///         $crate::platform::DeviceId::ZERO,
+///     ]);
+///     static DRIVER: $crate::platform::DriverVTable =
+///         $crate::platform::DriverVTable::new::<MikrobusDriver, 2>(&OF_TABLE);
+///     impl $crate::Module for Module {
+///         fn init(module: &'static ThisModule) -> Result<Self> {
+///             let reg =
+///                 $crate::platform::Registration::new(
+///                     ::core::pin::Pin::static_ref(&DRIVER), module);
+///             Ok(Module { _reg: reg })
+///         }
+///     }
+/// }
+///
+/// struct PlatformSimple;
+///
+/// #[vtable]
+/// impl platform::Driver for PlatformSimple {
+///     const NAME: &'static CStr = c_str!("PlatformSimple");
+/// }
+/// ```
+#[macro_export]
+macro_rules! module_platform_driver {
+    (@replace_expr $_t:tt $sub:expr) => {$sub};
+
+    (@count_devices $($x:expr),*) => {
+        0usize $(+ $crate::module_platform_driver!(@replace_expr $x 1usize))*
+    };
+
+    (driver: $driver:ident, of_table: [$($of_id:expr),+ $(,)?], $($f:tt)*) => {
+        struct Module {
+            _reg: $crate::platform::Registration,
+        }
+
+        $crate::prelude::module! {
+            type: Module,
+            $($f)*
+        }
+
+        const _: () = {
+            // SAFETY: C will not read off the end of this constant since the last element is zero.
+            static OF_TABLE: $crate::platform::DeviceIdTable<
+                {$crate::module_platform_driver!(@count_devices $($of_id),+) + 1} > =
+                $crate::platform::DeviceIdTable::new(
+                    [$($of_id.to_rawid()),*, $crate::platform::DeviceId::ZERO]);
+
+            static DRIVER: $crate::platform::DriverVTable =
+                $crate::platform::DriverVTable::new::<
+                    $driver, {$crate::module_platform_driver!(@count_devices $($of_id),+) + 1}
+                >(&OF_TABLE);
+
+            impl $crate::Module for Module {
+                fn init(module: &'static ThisModule) -> Result<Self> {
+                    let reg = $crate::platform::Registration::new(
+                        ::core::pin::Pin::static_ref(&DRIVER), module);
+                    Ok(Module { _reg: reg })
+                }
+            }
+        };
+    };
+}

-- 
2.46.0


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 2/8] dt-bindings: connector: Add MikorBUS connector
  2024-09-11 14:27 [PATCH 0/8] Add generic overlay for MikroBUS addon boards Ayush Singh
  2024-09-11 14:27 ` [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions Ayush Singh
@ 2024-09-11 14:27 ` Ayush Singh
  2024-09-11 15:26   ` Rob Herring (Arm)
  2024-09-11 17:26   ` Rob Herring
  2024-09-11 14:27 ` [PATCH 3/8] mikrobus: Add mikrobus driver Ayush Singh
                   ` (7 subsequent siblings)
  9 siblings, 2 replies; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 14:27 UTC (permalink / raw)
  To: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	Nishanth Menon, Vignesh Raghavendra, Tero Kristo
  Cc: linux-kernel, rust-for-linux, devicetree, linux-arm-kernel,
	Ayush Singh

Add DT bindings for mikroBUS interface. MikroBUS [0] is an open standard
developed by MikroElektronika for connecting add-on boards to
microcontrollers or microprocessors.

MikroBUS connector node will optionally act as nexus nodes for routing
GPIOs and PWM.

For GPIOs, the following pin numbering should be followed:

  0: PWM
  1: INT
  2: RX
  3: TX
  4: SCL
  5: SDA
  6: MOSI
  7: MISO
  8: SCK
  9: CS
  10: RST
  11: AN

For PWM, the PWM pin should be on channel 0.

I am not quite sure how to deal with the nexus node properties
(#gpio-cells, gpio-map, gpio-map-mask, gpio-map-pass-thru) since they
seem to conflict with upstream gpio schema (gpio-controller is a
dependency of #gpio-cells).

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

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 .../bindings/connector/mikrobus-connector.yaml     | 40 ++++++++++++++++++++++
 MAINTAINERS                                        |  5 +++
 2 files changed, 45 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
new file mode 100644
index 000000000000..603e4627076c
--- /dev/null
+++ b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+#
+# Copyright (c) Ayush Singh <ayush@beagleboard.org>
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/connector/mikrobus-connector.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: mikroBUS add-on board socket
+
+maintainers:
+  - Ayush Singh <ayush@beagleboard.org>
+
+properties:
+  compatible:
+    const: mikrobus-connector
+
+required:
+  - compatible
+
+additionalProperties: true
+
+examples:
+  - |
+    mikrobus_connector0: mikrobus-connector0 {
+      status = "okay";
+      compatible = "mikrobus-connector";
+
+      #gpio-cells = <2>;
+      gpio-map =
+      <0 0 &main_gpio1 11 0>, <1 0 &main_gpio1 9 0>,
+      <2 0 &main_gpio1 24 0>, <3 0 &main_gpio1 25 0>,
+      <4 0 &main_gpio1 22 0>, <5 0 &main_gpio1 23 0>,
+      <6 0 &main_gpio1 7 0>, <7 0 &main_gpio1 8 0>,
+      <8 0 &main_gpio1 14 0>, <9 0 &main_gpio1 13 0>,
+      <10 0 &main_gpio1 12 0>, <11 0 &main_gpio1 10 0>;
+      gpio-map-mask = <0xf 0x0>;
+      gpio-map-pass-thru = <0x0 0x1>;
+    };
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 0a3d9e17295a..0cc27446b18a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15429,6 +15429,11 @@ M:	Oliver Neukum <oliver@neukum.org>
 S:	Maintained
 F:	drivers/usb/image/microtek.*
 
+MIKROBUS CONNECTOR
+M:	Ayush Singh <ayush@beagleboard.org>
+S:	Maintained
+F:	Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
+
 MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT
 M:	Luka Kovacic <luka.kovacic@sartura.hr>
 M:	Luka Perkov <luka.perkov@sartura.hr>

-- 
2.46.0


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 3/8] mikrobus: Add mikrobus driver
  2024-09-11 14:27 [PATCH 0/8] Add generic overlay for MikroBUS addon boards Ayush Singh
  2024-09-11 14:27 ` [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions Ayush Singh
  2024-09-11 14:27 ` [PATCH 2/8] dt-bindings: connector: Add MikorBUS connector Ayush Singh
@ 2024-09-11 14:27 ` Ayush Singh
  2024-09-11 14:57   ` Greg Kroah-Hartman
  2024-09-11 15:48   ` Andrew Lunn
  2024-09-11 14:27 ` [PATCH 4/8] dts: ti: k3-am625-beagleplay: Enable mikroBUS connector Ayush Singh
                   ` (6 subsequent siblings)
  9 siblings, 2 replies; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 14:27 UTC (permalink / raw)
  To: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	Nishanth Menon, Vignesh Raghavendra, Tero Kristo
  Cc: linux-kernel, rust-for-linux, devicetree, linux-arm-kernel,
	Ayush Singh

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
+
+
+	  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");
+    }
+}

-- 
2.46.0


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 4/8] dts: ti: k3-am625-beagleplay: Enable mikroBUS connector
  2024-09-11 14:27 [PATCH 0/8] Add generic overlay for MikroBUS addon boards Ayush Singh
                   ` (2 preceding siblings ...)
  2024-09-11 14:27 ` [PATCH 3/8] mikrobus: Add mikrobus driver Ayush Singh
@ 2024-09-11 14:27 ` Ayush Singh
  2024-09-11 14:27 ` [PATCH 5/8] dts: ti: beagleplay: Add mikrobus connector symbols Ayush Singh
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 14:27 UTC (permalink / raw)
  To: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	Nishanth Menon, Vignesh Raghavendra, Tero Kristo
  Cc: linux-kernel, rust-for-linux, devicetree, linux-arm-kernel,
	Ayush Singh

Add mikroBUS connector support for Beagleplay. Acts as a nexus node for
gpios. Allows defining GPIOS relative to the connector. The pin
numbering is as follows:
  0: PWM
  1: INT
  2: RX
  3: TX
  4: SCL
  5: SDA
  6: MOSI
  7: MISO
  8: SCK
  9: CS
  10: RST
  11: AN

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts | 53 +++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 6 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..628bcfcc4651 100644
--- a/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
+++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay.dts
@@ -227,6 +227,21 @@ simple-audio-card,codec {
 		};
 	};
 
+	mikrobus_connector0: mikrobus-connector0 {
+		status = "disabled";
+		compatible = "mikrobus-connector";
+
+		#gpio-cells = <2>;
+		gpio-map =
+		<0 0 &main_gpio1 11 0>, <1 0 &main_gpio1 9 0>,
+		<2 0 &main_gpio1 24 0>, <3 0 &main_gpio1 25 0>,
+		<4 0 &main_gpio1 22 0>, <5 0 &main_gpio1 23 0>,
+		<6 0 &main_gpio1 7 0>, <7 0 &main_gpio1 8 0>,
+		<8 0 &main_gpio1 14 0>, <9 0 &main_gpio1 13 0>,
+		<10 0 &main_gpio1 12 0>, <11 0 &main_gpio1 10 0>;
+		gpio-map-mask = <0xf 0x0>;
+		gpio-map-pass-thru = <0x0 0x1>;
+	};
 };
 
 &main_pmx0 {
@@ -394,6 +409,25 @@ 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_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_uart_pins_default: mikrobus-uart-default-pins {
 		pinctrl-single,pins = <
 			AM62X_IOPAD(0x01d8, PIN_INPUT, 1) /* (C15) MCAN0_TX.UART5_RXD */
@@ -401,6 +435,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 */
@@ -804,10 +845,8 @@ it66121_out: endpoint {
 };
 
 &main_i2c3 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&mikrobus_i2c_pins_default>;
 	clock-frequency = <400000>;
-	status = "okay";
+	status = "disabled";
 };
 
 &main_spi2 {
@@ -876,9 +915,7 @@ &main_uart1 {
 };
 
 &main_uart5 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&mikrobus_uart_pins_default>;
-	status = "okay";
+	status = "disabled";
 };
 
 &main_uart6 {
@@ -925,3 +962,7 @@ &mcasp1 {
 	       0 0 0 0
 	>;
 };
+
+&ecap2 {
+	status = "okay";
+};

-- 
2.46.0


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 5/8] dts: ti: beagleplay: Add mikrobus connector symbols
  2024-09-11 14:27 [PATCH 0/8] Add generic overlay for MikroBUS addon boards Ayush Singh
                   ` (3 preceding siblings ...)
  2024-09-11 14:27 ` [PATCH 4/8] dts: ti: k3-am625-beagleplay: Enable mikroBUS connector Ayush Singh
@ 2024-09-11 14:27 ` Ayush Singh
  2024-09-11 14:27 ` [PATCH 6/8] addon_boards: Add addon_boards plumbing Ayush Singh
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 14:27 UTC (permalink / raw)
  To: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	Nishanth Menon, Vignesh Raghavendra, Tero Kristo
  Cc: linux-kernel, rust-for-linux, devicetree, linux-arm-kernel,
	Ayush Singh

- I2C, UART, PWM symbols
- MIKROBUS_GPIO defines a gpio-controller that defines the pins in the
  following order:
  0: PWM
  1: INT
  2: RX
  3: TX
  4: SCL
  5: SDA
  6: MOSI
  7: MISO
  8: SCK
  9: CS
  10: RST
  11: AN
- PWM should always use channel 0. Use nexus node for routing

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 MAINTAINERS                                        |  1 +
 arch/arm64/boot/dts/ti/Makefile                    |  1 +
 .../k3-am625-beagleplay-mikrobus-connector0.dtso   | 49 ++++++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d0c18bd7b558..95f228c85a40 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15432,6 +15432,7 @@ F:	drivers/usb/image/microtek.*
 MIKROBUS CONNECTOR
 M:	Ayush Singh <ayush@beagleboard.org>
 S:	Maintained
+F:	arch/arm64/boot/dts/ti/k3-am625-beagleplay-mikrobus-connector0.dtso
 F:	Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
 F:	drivers/misc/mikrobus.rs
 
diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index bcd392c3206e..c628954a357b 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -12,6 +12,7 @@
 dtb-$(CONFIG_ARCH_K3) += k3-am625-beagleplay.dtb
 dtb-$(CONFIG_ARCH_K3) += k3-am625-beagleplay-csi2-ov5640.dtbo
 dtb-$(CONFIG_ARCH_K3) += k3-am625-beagleplay-csi2-tevi-ov5640.dtbo
+dtb-$(CONFIG_ARCH_K3) += k3-am625-beagleplay-mikrobus-connector0.dtbo
 dtb-$(CONFIG_ARCH_K3) += k3-am625-phyboard-lyra-rdk.dtb
 dtb-$(CONFIG_ARCH_K3) += k3-am625-sk.dtb
 dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-nonwifi-dahlia.dtb
diff --git a/arch/arm64/boot/dts/ti/k3-am625-beagleplay-mikrobus-connector0.dtso b/arch/arm64/boot/dts/ti/k3-am625-beagleplay-mikrobus-connector0.dtso
new file mode 100644
index 000000000000..81d370249f64
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-am625-beagleplay-mikrobus-connector0.dtso
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/**
+ * MikroBUS Overlay for BeaglePlay MikroBUS Connector 0
+ *
+ * Copyright (C) 2024 Ayush Singh <ayush@beagleboard.org>
+ */
+
+/dts-v1/;
+/plugin/;
+
+&{/} {
+	__symbols__ {
+		MIKROBUS_CONNECTOR = "/mikrobus-connector0";
+
+		/* GPIO controller for all pins */
+		MIKROBUS_ALL_GPIO = "/mikrobus-connector0";
+
+		/* MikroBUS connector 0 SCL Pin options */
+		MIKROBUS_SCL_MUX_I2C_SCL = "/bus@f0000/pinctrl@f4000/mikrobus-i2c-default-pins";
+		MIKROBUS_SCL_MUX_DIGITAL = "/bus@f0000/pinctrl@f4000/mikrobus-i2c-gpio-pins";
+
+		/* MikroBUS connector 0 SDA Pin options */
+		MIKROBUS_SDA_MUX_I2C_SDA = "/bus@f0000/pinctrl@f4000/mikrobus-i2c-default-pins";
+		MIKROBUS_SDA_MUX_DIGITAL = "/bus@f0000/pinctrl@f4000/mikrobus-i2c-gpio-pins";
+
+		/* MikroBUS connector 0 UART_TX Pin options */
+		MIKROBUS_TX_MUX_UART_TX = "/bus@f0000/pinctrl@f4000/mikrobus-uart-default-pins";
+		MIKROBUS_TX_MUX_DIGITAL = "/bus@f0000/pinctrl@f4000/mikrobus-uart-gpio-pins";
+
+		/* MikroBUS connector 0 UART_RX Pin options */
+		MIKROBUS_RX_MUX_UART_RX = "/bus@f0000/pinctrl@f4000/mikrobus-uart-default-pins";
+		MIKROBUS_RX_MUX_DIGITAL = "/bus@f0000/pinctrl@f4000/mikrobus-uart-gpio-pins";
+
+		/* MikroBUS connector 0 PWM Pin options */
+		MIKROBUS_PWM_MUX_PWM = "/bus@f0000/pinctrl@f4000/mikrobus-pwm-default-pins";
+		MIKROBUS_PWM_MUX_DIGITAL = "/bus@f0000/pinctrl@f4000/mikrobus-pwm-gpio-pins";
+
+		/* MikroBUS connector 0 uses main_i2c3 for I2C on BeaglePlay */
+		MIKROBUS_SCL_I2C = "/bus@f0000/i2c@20030000";
+		MIKROBUS_SDA_I2C = "/bus@f0000/i2c@20030000";
+
+		/* MikroBUS connector 0 uses main_uart5 for UART on BeaglePlay */
+		MIKROBUS_TX_UART = "/bus@f0000/serial@2850000";
+		MIKROBUS_RX_UART = "/bus@f0000/serial@2850000";
+
+		/* MikroBUS connector 0 uses ecap2 for PWM on BeaglePlay */
+		MIKROBUS_PWM_PWM = "/bus@f0000/pwm@23120000";
+	};
+};

-- 
2.46.0


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 6/8] addon_boards: Add addon_boards plumbing
  2024-09-11 14:27 [PATCH 0/8] Add generic overlay for MikroBUS addon boards Ayush Singh
                   ` (4 preceding siblings ...)
  2024-09-11 14:27 ` [PATCH 5/8] dts: ti: beagleplay: Add mikrobus connector symbols Ayush Singh
@ 2024-09-11 14:27 ` Ayush Singh
  2024-09-11 15:00   ` Greg Kroah-Hartman
  2024-09-11 14:27 ` [PATCH 7/8] addon_boards: mikrobus: Add Weather Click Ayush Singh
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 14:27 UTC (permalink / raw)
  To: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	Nishanth Menon, Vignesh Raghavendra, Tero Kristo
  Cc: linux-kernel, rust-for-linux, devicetree, linux-arm-kernel,
	Ayush Singh

A directory to store and build addon_board overlays like mikroBUS,
Groove, etc. The overlays present here should be completely independent
of the underlying connector.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 Kbuild                         |  1 +
 Kconfig                        |  2 ++
 MAINTAINERS                    |  1 +
 addon_boards/Kconfig           | 16 ++++++++++++++++
 addon_boards/Makefile          |  3 +++
 addon_boards/mikrobus/Makefile |  1 +
 6 files changed, 24 insertions(+)

diff --git a/Kbuild b/Kbuild
index 464b34a08f51..9c897397f55f 100644
--- a/Kbuild
+++ b/Kbuild
@@ -97,3 +97,4 @@ obj-$(CONFIG_SAMPLES)	+= samples/
 obj-$(CONFIG_NET)	+= net/
 obj-y			+= virt/
 obj-y			+= $(ARCH_DRIVERS)
+obj-y			+= addon_boards/
diff --git a/Kconfig b/Kconfig
index 745bc773f567..49880d4e91e9 100644
--- a/Kconfig
+++ b/Kconfig
@@ -30,3 +30,5 @@ source "lib/Kconfig"
 source "lib/Kconfig.debug"
 
 source "Documentation/Kconfig"
+
+source "addon_boards/Kconfig"
diff --git a/MAINTAINERS b/MAINTAINERS
index 95f228c85a40..8e2e0f8d16be 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15432,6 +15432,7 @@ F:	drivers/usb/image/microtek.*
 MIKROBUS CONNECTOR
 M:	Ayush Singh <ayush@beagleboard.org>
 S:	Maintained
+F:	addon_boards/mikrobus/*.dtso
 F:	arch/arm64/boot/dts/ti/k3-am625-beagleplay-mikrobus-connector0.dtso
 F:	Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
 F:	drivers/misc/mikrobus.rs
diff --git a/addon_boards/Kconfig b/addon_boards/Kconfig
new file mode 100644
index 000000000000..01766ab28848
--- /dev/null
+++ b/addon_boards/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0
+
+menu "Addon Board Overlays"
+
+config MIKROBUS_BOARD_OVERLAYS
+	bool "mikroBUS board overlays"
+	depends on OF_OVERLAY
+	depends on MIKROBUS
+	help
+	  This option enables the mikroBUS addon board overlays. mikroBUS is an
+	  add-on board socket standard that offers maximum expandability with
+	  the smallest number of pins.
+
+	  Say Y here to enable overlays for MikroBUS addon boards.
+
+endmenu
diff --git a/addon_boards/Makefile b/addon_boards/Makefile
new file mode 100644
index 000000000000..38275c3ff4c1
--- /dev/null
+++ b/addon_boards/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+subdir-$(CONFIG_MIKROBUS_BOARD_OVERLAYS) += mikrobus
diff --git a/addon_boards/mikrobus/Makefile b/addon_boards/mikrobus/Makefile
new file mode 100644
index 000000000000..f66554cd5c45
--- /dev/null
+++ b/addon_boards/mikrobus/Makefile
@@ -0,0 +1 @@
+# SPDX-License-Identifier: GPL-2.0

-- 
2.46.0


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 7/8] addon_boards: mikrobus: Add Weather Click
  2024-09-11 14:27 [PATCH 0/8] Add generic overlay for MikroBUS addon boards Ayush Singh
                   ` (5 preceding siblings ...)
  2024-09-11 14:27 ` [PATCH 6/8] addon_boards: Add addon_boards plumbing Ayush Singh
@ 2024-09-11 14:27 ` Ayush Singh
  2024-09-11 14:27 ` [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click Ayush Singh
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 14:27 UTC (permalink / raw)
  To: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	Nishanth Menon, Vignesh Raghavendra, Tero Kristo
  Cc: linux-kernel, rust-for-linux, devicetree, linux-arm-kernel,
	Ayush Singh

An I2C MikroBUS addon board containing BME280 sensor.

The same board also optionally supports SPI. Hence the i2c suffix.

Link: https://www.mikroe.com/weather-click Weather Click

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

diff --git a/addon_boards/mikrobus/Makefile b/addon_boards/mikrobus/Makefile
index f66554cd5c45..4c7a68ea9504 100644
--- a/addon_boards/mikrobus/Makefile
+++ b/addon_boards/mikrobus/Makefile
@@ -1 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
+
+dtb-y += mikroe-5761-i2c.dtbo
diff --git a/addon_boards/mikrobus/mikroe-5761-i2c.dtso b/addon_boards/mikrobus/mikroe-5761-i2c.dtso
new file mode 100644
index 000000000000..1111d3f7147f
--- /dev/null
+++ b/addon_boards/mikrobus/mikroe-5761-i2c.dtso
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/**
+ * MikroBUS - Weather Click I2C
+ *
+ * https://www.mikroe.com/weather-click
+ *
+ * Copyright (C) 2024 Ayush Singh <ayush@beagleboard.org>
+ */
+
+/dts-v1/;
+/plugin/;
+
+&MIKROBUS_CONNECTOR {
+	status = "okay";
+};
+
+&MIKROBUS_SCL_I2C {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&MIKROBUS_SDA_MUX_I2C_SDA>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	bme280@76 {
+		compatible = "bosch,bme280";
+		reg = <0x76>;
+	};
+};

-- 
2.46.0


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click
  2024-09-11 14:27 [PATCH 0/8] Add generic overlay for MikroBUS addon boards Ayush Singh
                   ` (6 preceding siblings ...)
  2024-09-11 14:27 ` [PATCH 7/8] addon_boards: mikrobus: Add Weather Click Ayush Singh
@ 2024-09-11 14:27 ` Ayush Singh
  2024-09-11 14:58   ` Greg Kroah-Hartman
  2024-09-11 17:02 ` [PATCH 0/8] Add generic overlay for MikroBUS addon boards Rob Herring
  2024-09-13 10:05 ` Alexander Stein
  9 siblings, 1 reply; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 14:27 UTC (permalink / raw)
  To: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	Nishanth Menon, Vignesh Raghavendra, Tero Kristo
  Cc: linux-kernel, rust-for-linux, devicetree, linux-arm-kernel,
	Ayush Singh

- 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(+)

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>;
+	};
+};

-- 
2.46.0


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions
  2024-09-11 14:27 ` [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions Ayush Singh
@ 2024-09-11 14:56   ` Greg Kroah-Hartman
  2024-09-11 15:52     ` Ayush Singh
  2024-09-11 17:35     ` Danilo Krummrich
  0 siblings, 2 replies; 39+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-11 14:56 UTC (permalink / raw)
  To: Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

On Wed, Sep 11, 2024 at 07:57:18PM +0530, Ayush Singh wrote:
> +/// An identifier for Platform devices.
> +///
> +/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
> +/// Platform driver.
> +///
> +/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
> +pub struct DeviceId(&'static CStr);
> +
> +impl DeviceId {

<snip>

I appreciate posting this, but this really should go on top of the
device driver work Danilo Krummrich has been doing.  He and I spent a
lot of time working through this this past weekend (well, him talking
and explaining, and me asking too many stupid questions...)

I think what he has will make the platform driver/device work simpler
here, and I'll be glad to take it based on that, this "independent" code
that doesn't interact with that isn't the best idea overall.

It also will properly handle the "Driver" interaction as well, which we
need to get right, not a one-off like this for a platform driver.
Hopefully that will not cause much, if any, changes for your use of this
in your driver, but let's see.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 3/8] mikrobus: Add mikrobus driver
  2024-09-11 14:27 ` [PATCH 3/8] mikrobus: Add mikrobus driver Ayush Singh
@ 2024-09-11 14:57   ` Greg Kroah-Hartman
  2024-09-11 16:02     ` Ayush Singh
  2024-09-11 15:48   ` Andrew Lunn
  1 sibling, 1 reply; 39+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-11 14:57 UTC (permalink / raw)
  To: Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click
  2024-09-11 14:27 ` [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click Ayush Singh
@ 2024-09-11 14:58   ` Greg Kroah-Hartman
  2024-09-11 15:56     ` Ayush Singh
  0 siblings, 1 reply; 39+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-11 14:58 UTC (permalink / raw)
  To: Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 6/8] addon_boards: Add addon_boards plumbing
  2024-09-11 14:27 ` [PATCH 6/8] addon_boards: Add addon_boards plumbing Ayush Singh
@ 2024-09-11 15:00   ` Greg Kroah-Hartman
  2024-09-11 16:09     ` Ayush Singh
  0 siblings, 1 reply; 39+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-11 15:00 UTC (permalink / raw)
  To: Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

On Wed, Sep 11, 2024 at 07:57:23PM +0530, Ayush Singh wrote:
> A directory to store and build addon_board overlays like mikroBUS,
> Groove, etc. The overlays present here should be completely independent
> of the underlying connector.
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> ---
>  Kbuild                         |  1 +
>  Kconfig                        |  2 ++
>  MAINTAINERS                    |  1 +
>  addon_boards/Kconfig           | 16 ++++++++++++++++
>  addon_boards/Makefile          |  3 +++
>  addon_boards/mikrobus/Makefile |  1 +
>  6 files changed, 24 insertions(+)

Ah, here's where you add this.

It should be below drivers/ right?  But what's wrong with drivers/soc/?
Why is this so special?  Why not just under
drivers/microbus/addon_boards/ ?  Why is this tiny bus/device so
different from everything else out there?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/8] dt-bindings: connector: Add MikorBUS connector
  2024-09-11 14:27 ` [PATCH 2/8] dt-bindings: connector: Add MikorBUS connector Ayush Singh
@ 2024-09-11 15:26   ` Rob Herring (Arm)
  2024-09-11 17:26   ` Rob Herring
  1 sibling, 0 replies; 39+ messages in thread
From: Rob Herring (Arm) @ 2024-09-11 15:26 UTC (permalink / raw)
  To: Ayush Singh
  Cc: Alex Gaynor, Derek Kiernan, Arnd Bergmann, Alice Ryhl,
	Benno Lossin, Greg Kroah-Hartman, robertcnelson, d-gole,
	linux-kernel, Björn Roy Baron, jkridner, Tero Kristo,
	Vignesh Raghavendra, Miguel Ojeda, rust-for-linux, Gary Guo,
	fabien.parent, Dragan Cvetic, Andrew Davis, Conor Dooley,
	Nishanth Menon, Andreas Hindborg, lorforlinux, Boqun Feng,
	Trevor Gross, linux-arm-kernel, devicetree, Krzysztof Kozlowski


On Wed, 11 Sep 2024 19:57:19 +0530, Ayush Singh wrote:
> Add DT bindings for mikroBUS interface. MikroBUS [0] is an open standard
> developed by MikroElektronika for connecting add-on boards to
> microcontrollers or microprocessors.
> 
> MikroBUS connector node will optionally act as nexus nodes for routing
> GPIOs and PWM.
> 
> For GPIOs, the following pin numbering should be followed:
> 
>   0: PWM
>   1: INT
>   2: RX
>   3: TX
>   4: SCL
>   5: SDA
>   6: MOSI
>   7: MISO
>   8: SCK
>   9: CS
>   10: RST
>   11: AN
> 
> For PWM, the PWM pin should be on channel 0.
> 
> I am not quite sure how to deal with the nexus node properties
> (#gpio-cells, gpio-map, gpio-map-mask, gpio-map-pass-thru) since they
> seem to conflict with upstream gpio schema (gpio-controller is a
> dependency of #gpio-cells).
> 
> [0]: https://www.mikroe.com/
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> ---
>  .../bindings/connector/mikrobus-connector.yaml     | 40 ++++++++++++++++++++++
>  MAINTAINERS                                        |  5 +++
>  2 files changed, 45 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/connector/mikrobus-connector.example.dtb: mikrobus-connector0: 'gpio-controller' is a dependency of '#gpio-cells'
	from schema $id: http://devicetree.org/schemas/gpio/gpio.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240911-mikrobus-dt-v1-2-3ded4dc879e7@beagleboard.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 3/8] mikrobus: Add mikrobus driver
  2024-09-11 14:27 ` [PATCH 3/8] mikrobus: Add mikrobus driver Ayush Singh
  2024-09-11 14:57   ` Greg Kroah-Hartman
@ 2024-09-11 15:48   ` Andrew Lunn
  1 sibling, 0 replies; 39+ messages in thread
From: Andrew Lunn @ 2024-09-11 15:48 UTC (permalink / raw)
  To: Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	Nishanth Menon, Vignesh Raghavendra, Tero Kristo, linux-kernel,
	rust-for-linux, devicetree, linux-arm-kernel

> --- 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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions
  2024-09-11 14:56   ` Greg Kroah-Hartman
@ 2024-09-11 15:52     ` Ayush Singh
  2024-09-11 17:39       ` Danilo Krummrich
  2024-09-11 17:35     ` Danilo Krummrich
  1 sibling, 1 reply; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 15:52 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel


On 9/11/24 20:26, Greg Kroah-Hartman wrote:
> On Wed, Sep 11, 2024 at 07:57:18PM +0530, Ayush Singh wrote:
>> +/// An identifier for Platform devices.
>> +///
>> +/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
>> +/// Platform driver.
>> +///
>> +/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
>> +pub struct DeviceId(&'static CStr);
>> +
>> +impl DeviceId {
> <snip>
>
> I appreciate posting this, but this really should go on top of the
> device driver work Danilo Krummrich has been doing.  He and I spent a
> lot of time working through this this past weekend (well, him talking
> and explaining, and me asking too many stupid questions...)
>
> I think what he has will make the platform driver/device work simpler
> here, and I'll be glad to take it based on that, this "independent" code
> that doesn't interact with that isn't the best idea overall.
>
> It also will properly handle the "Driver" interaction as well, which we
> need to get right, not a one-off like this for a platform driver.
> Hopefully that will not cause much, if any, changes for your use of this
> in your driver, but let's see.
>
> thanks,
>
> greg k-h
>
Sure, can you provide me a link to patches or maybe the branch 
containing that work? I also think it would be good to have the link in 
Zulip discussion for Platform Device and Driver.


Ayush Singh


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click
  2024-09-11 14:58   ` Greg Kroah-Hartman
@ 2024-09-11 15:56     ` Ayush Singh
  2024-09-11 20:04       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 15:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 3/8] mikrobus: Add mikrobus driver
  2024-09-11 14:57   ` Greg Kroah-Hartman
@ 2024-09-11 16:02     ` Ayush Singh
  2024-09-11 20:03       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 16:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 6/8] addon_boards: Add addon_boards plumbing
  2024-09-11 15:00   ` Greg Kroah-Hartman
@ 2024-09-11 16:09     ` Ayush Singh
  0 siblings, 0 replies; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 16:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

On 9/11/24 20:30, Greg Kroah-Hartman wrote:

> On Wed, Sep 11, 2024 at 07:57:23PM +0530, Ayush Singh wrote:
>> A directory to store and build addon_board overlays like mikroBUS,
>> Groove, etc. The overlays present here should be completely independent
>> of the underlying connector.
>>
>> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
>> ---
>>   Kbuild                         |  1 +
>>   Kconfig                        |  2 ++
>>   MAINTAINERS                    |  1 +
>>   addon_boards/Kconfig           | 16 ++++++++++++++++
>>   addon_boards/Makefile          |  3 +++
>>   addon_boards/mikrobus/Makefile |  1 +
>>   6 files changed, 24 insertions(+)
> Ah, here's where you add this.
>
> It should be below drivers/ right?  But what's wrong with drivers/soc/?
> Why is this so special?  Why not just under
> drivers/microbus/addon_boards/ ?  Why is this tiny bus/device so
> different from everything else out there?
>
> thanks,
>
> greg k-h


Well, it can go under drivers for mikrobus, but there will be other 
addon board connectors which will not need any kind of driver. In fact, 
the original patch series this is based on [0] did not have any driver 
for the connector.


As for bus, see my reply to the other patch in the series [1]. 
Basically, while the name of the standard itself is "mikroBUS", it is 
not really a "bus" in Linux sense.


[0]: 
https://lore.kernel.org/linux-arm-kernel/20240702164403.29067-1-afd@ti.com/

[1]: 
https://lore.kernel.org/all/ecd1fff8-9c15-496a-982f-36e6c58e906a@gmail.com/


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/8] Add generic overlay for MikroBUS addon boards
  2024-09-11 14:27 [PATCH 0/8] Add generic overlay for MikroBUS addon boards Ayush Singh
                   ` (7 preceding siblings ...)
  2024-09-11 14:27 ` [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click Ayush Singh
@ 2024-09-11 17:02 ` Rob Herring
  2024-09-20 16:51   ` Ayush Singh
  2024-09-13 10:05 ` Alexander Stein
  9 siblings, 1 reply; 39+ messages in thread
From: Rob Herring @ 2024-09-11 17:02 UTC (permalink / raw)
  To: Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

On Wed, Sep 11, 2024 at 07:57:17PM +0530, Ayush Singh wrote:
> Hello all,
> 
> This is an attempt to add MikroBUS addon support using the approach
> described by Grove connector patch series [0].
> 
> The patch series tests out 2 addon boards + pwm and GPIO on the MikroBUS
> connector. The boards used are GPS3 Click (for UART) [1] and Weather
> Click (for I2C) [2]. Additionally, I have tested relative GPIO numbering
> using devicetree nexus nodes [3].
> 
> The patch series does not attempt to do any dynamic discovery for 1-wire
> eeprom MikroBUS addon boards, nor does it provide any sysfs entry for
> board addition/removal. The connector driver might include them after
> the basic support is ironed out, but the existing patches for dynamic
> overlays work fine.
> 
> The patch series has been tested on BeaglePlay [4].
> 
> I will now go over individual parts of the patch series, but for a
> better picture of the approach, it would be best to checkout [0] first.
> 
> MikroBUS connector driver
> -------------------------
> 
> Just a stub platform driver for now. Will be extended in the future for
> dynamic board discovery using 1-wire eeprom present in some MikroBUS
> addon boards.
> 
> While it is a stub driver, disabling it will make the GPIO connector
> nexus node unreachable (any driver attempting to use it will enter
> differed probing). So it is required.
> 
> MikroBUS connector Devicetree
> ------------------------------
> 
> The connector devicetree defines the MikroBUS GPIO nexus node. This
> allows using pin numbering relative to MikroBUS connector pins in the
> addon boards overlay. Currently, the patch uses a clockwise numbering:
> 
>   0: PWM
>   1: INT
>   2: RX
>   3: TX
>   4: SCL
>   5: SDA
>   6: MOSI
>   7: MISO
>   8: SCK
>   9: CS
>   10: RST
>   11: AN
> 
> Additionally, in case PWM pin is not using channel 0, a nexus node for pwm
> should also be used and go in the connector devicetree.
> 
> MikroBUS connector symbols overlay
> ----------------------------------
> 
> To make an overlay generic we need a standard name scheme which we
> use across base boards. For the connector pins the pinmux phandle
> shall be:
> 
> <connector-name>_<pin-name>_mux_<pin-function>
> 
> For the parent provider phandle, we use a similar naming scheme:
> 
> <connector-name>_<pin-name>_<pin-function>
> 
> For GPIO controller, I am using `MIKROBUS_GPIO` name since with nexus
> nodes, we do not need to define individual pin gpio controllers.
> 
> The string symbols can be replaced with phandles once [5] is accepted.
> That will make connector stacking much simpler.
> 
> MikroBUS addon board overlay
> ----------------------------
> 
> The patch puts the addon board overlays in their own directory. I am
> using the following naming scheme for MikroBUS addon boards:
> 
> <vendor>-<product_id>.dtso
> 
> Mikroe [6] lists this for all boards in their website, but I am not sure
> if other vendors have a product_id.
> 
> This naming also makes future dynamic discovery easier, since click id
> spec [7] contains vendor_id and product_id in the header.
> 
> Usage
> -----
> 
> So what does this all look like? Let's take an example of a BeaglePlay
> with one MikroBUS connectors for which we have physically attached a
> Wather click module to the first connector. Doing ahead of time
> command-line DT overlay application:
> 
> ./fdtoverlay \
> 	-o output.dtb \
> 	-i k3-am625-beagleplay.dtb \
> 		k3-am625-beagleplay-mikrobus-connector0.dtbo mikroe-5761.dtbo
> 
> Open Items
> ----------
> 
> - SPI Support: 
>   Currently SPI dt requires `reg` property to specify the
>   chipselect to use. This, makes the SPI device overlay dependent on the
>   SPI controller. Thus for SPI support, we need a way to allow defining
>   SPI chipselect relative to MikroBUS pins, and not the actual device
>   controller.
> 
>   One possible solution is to introduce something like `named-reg` and
>   allow selecting the chipselect by string array. But this probably
>   requires more discussion so I have left out SPI support for now.
> 
>   NOTE: pins other than the CS MikroBUS pin can be used as chipselect.
>   See [8].
> 
> - Controller symbol duplication
>   The current symbol scheme has multiple symbols for the same underlying
>   controller (Eg: MIKROBUS_SCL_MUX_I2C_SCL and MIKROBUS_SDA_MUX_I2C_SDA)
>   point to the same i2c controller.
> 
>   I think both of them will always use the same i2c controller, but
>   maybe there are some exceptions? So I have left it as it is for this
>   patch series. The same thing also applies to UART and SPI.
> 
>   NOTE: with the use of nexus node for GPIO, the GPIO controller symbol
>   will be the same for all pins.
> 
> - Nexus node dt-bindings
>   I am not quite sure how to deal with the nexus node properties
>   (#gpio-cells, gpio-map, gpio-map-mask, gpio-map-pass-thru) since they
>   seem to conflict with upstream gpio schema (gpio-controller is a
>   dependency of #gpio-cells).

Please submit a fix to dtschema. Either GH PR or patch to 
devicetree-spec list.

Rob

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/8] dt-bindings: connector: Add MikorBUS connector
  2024-09-11 14:27 ` [PATCH 2/8] dt-bindings: connector: Add MikorBUS connector Ayush Singh
  2024-09-11 15:26   ` Rob Herring (Arm)
@ 2024-09-11 17:26   ` Rob Herring
  2024-09-11 18:00     ` Ayush Singh
  1 sibling, 1 reply; 39+ messages in thread
From: Rob Herring @ 2024-09-11 17:26 UTC (permalink / raw)
  To: Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

On Wed, Sep 11, 2024 at 07:57:19PM +0530, Ayush Singh wrote:
> Add DT bindings for mikroBUS interface. MikroBUS [0] is an open standard
> developed by MikroElektronika for connecting add-on boards to
> microcontrollers or microprocessors.

Typo in the subject...

Isn't this v6? Where's the revision history?

> 
> MikroBUS connector node will optionally act as nexus nodes for routing
> GPIOs and PWM.
> 
> For GPIOs, the following pin numbering should be followed:
> 
>   0: PWM
>   1: INT
>   2: RX
>   3: TX
>   4: SCL
>   5: SDA
>   6: MOSI
>   7: MISO
>   8: SCK
>   9: CS
>   10: RST
>   11: AN
> 
> For PWM, the PWM pin should be on channel 0.
> 
> I am not quite sure how to deal with the nexus node properties
> (#gpio-cells, gpio-map, gpio-map-mask, gpio-map-pass-thru) since they
> seem to conflict with upstream gpio schema (gpio-controller is a
> dependency of #gpio-cells).
> 
> [0]: https://www.mikroe.com/
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> ---
>  .../bindings/connector/mikrobus-connector.yaml     | 40 ++++++++++++++++++++++
>  MAINTAINERS                                        |  5 +++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
> new file mode 100644
> index 000000000000..603e4627076c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +#
> +# Copyright (c) Ayush Singh <ayush@beagleboard.org>
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/connector/mikrobus-connector.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: mikroBUS add-on board socket
> +
> +maintainers:
> +  - Ayush Singh <ayush@beagleboard.org>
> +
> +properties:
> +  compatible:
> +    const: mikrobus-connector
> +
> +required:
> +  - compatible
> +
> +additionalProperties: true

Cannot be true. You're schema must be complete. I don't understand what 
happened to everything else in the binding.

> +
> +examples:
> +  - |
> +    mikrobus_connector0: mikrobus-connector0 {
> +      status = "okay";
> +      compatible = "mikrobus-connector";
> +
> +      #gpio-cells = <2>;
> +      gpio-map =
> +      <0 0 &main_gpio1 11 0>, <1 0 &main_gpio1 9 0>,
> +      <2 0 &main_gpio1 24 0>, <3 0 &main_gpio1 25 0>,
> +      <4 0 &main_gpio1 22 0>, <5 0 &main_gpio1 23 0>,
> +      <6 0 &main_gpio1 7 0>, <7 0 &main_gpio1 8 0>,
> +      <8 0 &main_gpio1 14 0>, <9 0 &main_gpio1 13 0>,
> +      <10 0 &main_gpio1 12 0>, <11 0 &main_gpio1 10 0>;
> +      gpio-map-mask = <0xf 0x0>;
> +      gpio-map-pass-thru = <0x0 0x1>;
> +    };
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0a3d9e17295a..0cc27446b18a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15429,6 +15429,11 @@ M:	Oliver Neukum <oliver@neukum.org>
>  S:	Maintained
>  F:	drivers/usb/image/microtek.*
>  
> +MIKROBUS CONNECTOR
> +M:	Ayush Singh <ayush@beagleboard.org>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
> +
>  MIKROTIK CRS3XX 98DX3236 BOARD SUPPORT
>  M:	Luka Kovacic <luka.kovacic@sartura.hr>
>  M:	Luka Perkov <luka.perkov@sartura.hr>
> 
> -- 
> 2.46.0
> 

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions
  2024-09-11 14:56   ` Greg Kroah-Hartman
  2024-09-11 15:52     ` Ayush Singh
@ 2024-09-11 17:35     ` Danilo Krummrich
  2024-09-11 20:07       ` Greg Kroah-Hartman
  2024-09-14 10:11       ` Janne Grunau
  1 sibling, 2 replies; 39+ messages in thread
From: Danilo Krummrich @ 2024-09-11 17:35 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Ayush Singh, fabien.parent, d-gole, lorforlinux, jkridner,
	robertcnelson, Andrew Davis, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic,
	Arnd Bergmann, Nishanth Menon, Vignesh Raghavendra, Tero Kristo,
	linux-kernel, rust-for-linux, devicetree, linux-arm-kernel

On Wed, Sep 11, 2024 at 04:56:14PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 11, 2024 at 07:57:18PM +0530, Ayush Singh wrote:
> > +/// An identifier for Platform devices.
> > +///
> > +/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
> > +/// Platform driver.
> > +///
> > +/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
> > +pub struct DeviceId(&'static CStr);
> > +
> > +impl DeviceId {
> 
> <snip>
> 
> I appreciate posting this, but this really should go on top of the
> device driver work Danilo Krummrich has been doing.

If everyone agrees, I'd offer to just provide platform device / driver
abstractions with my next patch series. This way you don't need to worry
about aligning things with the rest of the abstractions yourself and throughout
potential further versions of the series.

Just be aware that I probably won't get to work on it until after LPC.

> He and I spent a
> lot of time working through this this past weekend (well, him talking
> and explaining, and me asking too many stupid questions...)
> 
> I think what he has will make the platform driver/device work simpler
> here, and I'll be glad to take it based on that, this "independent" code
> that doesn't interact with that isn't the best idea overall.
> 
> It also will properly handle the "Driver" interaction as well, which we
> need to get right, not a one-off like this for a platform driver.
> Hopefully that will not cause much, if any, changes for your use of this
> in your driver, but let's see.
> 
> thanks,
> 
> greg k-h
> 

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions
  2024-09-11 15:52     ` Ayush Singh
@ 2024-09-11 17:39       ` Danilo Krummrich
  2024-09-11 18:05         ` Ayush Singh
  0 siblings, 1 reply; 39+ messages in thread
From: Danilo Krummrich @ 2024-09-11 17:39 UTC (permalink / raw)
  To: Ayush Singh
  Cc: Greg Kroah-Hartman, Ayush Singh, fabien.parent, d-gole,
	lorforlinux, jkridner, robertcnelson, Andrew Davis, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Nishanth Menon, Vignesh Raghavendra,
	Tero Kristo, linux-kernel, rust-for-linux, devicetree,
	linux-arm-kernel

On 9/11/24 5:52 PM, Ayush Singh wrote:
> Sure, can you provide me a link to patches or maybe the branch containing that 
> work? I also think it would be good to have the link in Zulip discussion for 
> Platform Device and Driver.

Sure, please see [1]. But please be aware that I plan to rework some parts
before sending out the next version.

[1] https://github.com/Rust-for-Linux/linux/tree/staging/rust-device

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 2/8] dt-bindings: connector: Add MikorBUS connector
  2024-09-11 17:26   ` Rob Herring
@ 2024-09-11 18:00     ` Ayush Singh
  0 siblings, 0 replies; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 18:00 UTC (permalink / raw)
  To: Rob Herring
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

On 9/11/24 22:56, Rob Herring wrote:

> On Wed, Sep 11, 2024 at 07:57:19PM +0530, Ayush Singh wrote:
>> Add DT bindings for mikroBUS interface. MikroBUS [0] is an open standard
>> developed by MikroElektronika for connecting add-on boards to
>> microcontrollers or microprocessors.
> Typo in the subject...
>
> Isn't this v6? Where's the revision history?

Well, at this point, it almost has nothing in common with [0] (the 
previous patch series was about making mikroBUS a Linux bus) and is more 
of a continuation of [1]. So I thought it would be better to treat it as 
a new patch series.

>> MikroBUS connector node will optionally act as nexus nodes for routing
>> GPIOs and PWM.
>>
>> For GPIOs, the following pin numbering should be followed:
>>
>>    0: PWM
>>    1: INT
>>    2: RX
>>    3: TX
>>    4: SCL
>>    5: SDA
>>    6: MOSI
>>    7: MISO
>>    8: SCK
>>    9: CS
>>    10: RST
>>    11: AN
>>
>> For PWM, the PWM pin should be on channel 0.
>>
>> I am not quite sure how to deal with the nexus node properties
>> (#gpio-cells, gpio-map, gpio-map-mask, gpio-map-pass-thru) since they
>> seem to conflict with upstream gpio schema (gpio-controller is a
>> dependency of #gpio-cells).
>>
>> [0]: https://www.mikroe.com/
>>
>> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
>> ---
>>   .../bindings/connector/mikrobus-connector.yaml     | 40 ++++++++++++++++++++++
>>   MAINTAINERS                                        |  5 +++
>>   2 files changed, 45 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
>> new file mode 100644
>> index 000000000000..603e4627076c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/connector/mikrobus-connector.yaml
>> @@ -0,0 +1,40 @@
>> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
>> +#
>> +# Copyright (c) Ayush Singh <ayush@beagleboard.org>
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/connector/mikrobus-connector.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: mikroBUS add-on board socket
>> +
>> +maintainers:
>> +  - Ayush Singh <ayush@beagleboard.org>
>> +
>> +properties:
>> +  compatible:
>> +    const: mikrobus-connector
>> +
>> +required:
>> +  - compatible
>> +
>> +additionalProperties: true
> Cannot be true. You're schema must be complete. I don't understand what
> happened to everything else in the binding.


So the current dtschema makes `gpio-controller` dependency of 
`#gpio-cells` which should not hold true for nexus node [2]. I also 
wanted to understand if the nexus node schema should go in upstream 
dtschema or be in kernel tree.

I will try to figure out how nexus node properties should look by taking 
interrupt nexus node bindings as a starting point.


[0]: 
https://lore.kernel.org/linux-arm-kernel/20240627-mikrobus-scratch-spi-v5-0-9e6c148bf5f0@beagleboard.org/

[1]: 
https://lore.kernel.org/linux-arm-kernel/20240702164403.29067-1-afd@ti.com/

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


Ayush Singh


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions
  2024-09-11 17:39       ` Danilo Krummrich
@ 2024-09-11 18:05         ` Ayush Singh
  2024-09-11 21:52           ` Danilo Krummrich
  0 siblings, 1 reply; 39+ messages in thread
From: Ayush Singh @ 2024-09-11 18:05 UTC (permalink / raw)
  To: Danilo Krummrich, Ayush Singh
  Cc: Greg Kroah-Hartman, fabien.parent, d-gole, lorforlinux, jkridner,
	robertcnelson, Andrew Davis, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic,
	Arnd Bergmann, Nishanth Menon, Vignesh Raghavendra, Tero Kristo,
	linux-kernel, rust-for-linux, devicetree, linux-arm-kernel

On 9/11/24 23:09, Danilo Krummrich wrote:

> On 9/11/24 5:52 PM, Ayush Singh wrote:
>> Sure, can you provide me a link to patches or maybe the branch 
>> containing that work? I also think it would be good to have the link 
>> in Zulip discussion for Platform Device and Driver.
>
> Sure, please see [1]. But please be aware that I plan to rework some 
> parts
> before sending out the next version.
>
> [1] https://github.com/Rust-for-Linux/linux/tree/staging/rust-device


Maybe the branch is just out of date? It still contains the generic 
structures for IdArray, IdTable and RawDeviceId.

Has something changed since the discussion here [0]?


[0]: 
https://lore.kernel.org/rust-for-linux/2024062031-untracked-opt-3ff1@gregkh/


Ayush Singh


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 3/8] mikrobus: Add mikrobus driver
  2024-09-11 16:02     ` Ayush Singh
@ 2024-09-11 20:03       ` Greg Kroah-Hartman
  2024-09-12 13:32         ` Ayush Singh
  0 siblings, 1 reply; 39+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-11 20:03 UTC (permalink / raw)
  To: Ayush Singh
  Cc: Ayush Singh, fabien.parent, d-gole, lorforlinux, jkridner,
	robertcnelson, Andrew Davis, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic,
	Arnd Bergmann, Nishanth Menon, Vignesh Raghavendra, Tero Kristo,
	linux-kernel, rust-for-linux, devicetree, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click
  2024-09-11 15:56     ` Ayush Singh
@ 2024-09-11 20:04       ` Greg Kroah-Hartman
  2024-09-12  7:16         ` Ayush Singh
  0 siblings, 1 reply; 39+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-11 20:04 UTC (permalink / raw)
  To: Ayush Singh
  Cc: Ayush Singh, fabien.parent, d-gole, lorforlinux, jkridner,
	robertcnelson, Andrew Davis, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic,
	Arnd Bergmann, Nishanth Menon, Vignesh Raghavendra, Tero Kristo,
	linux-kernel, rust-for-linux, devicetree, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions
  2024-09-11 17:35     ` Danilo Krummrich
@ 2024-09-11 20:07       ` Greg Kroah-Hartman
  2024-09-14 10:11       ` Janne Grunau
  1 sibling, 0 replies; 39+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-11 20:07 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Ayush Singh, fabien.parent, d-gole, lorforlinux, jkridner,
	robertcnelson, Andrew Davis, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic,
	Arnd Bergmann, Nishanth Menon, Vignesh Raghavendra, Tero Kristo,
	linux-kernel, rust-for-linux, devicetree, linux-arm-kernel

On Wed, Sep 11, 2024 at 07:35:35PM +0200, Danilo Krummrich wrote:
> On Wed, Sep 11, 2024 at 04:56:14PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 11, 2024 at 07:57:18PM +0530, Ayush Singh wrote:
> > > +/// An identifier for Platform devices.
> > > +///
> > > +/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
> > > +/// Platform driver.
> > > +///
> > > +/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
> > > +pub struct DeviceId(&'static CStr);
> > > +
> > > +impl DeviceId {
> > 
> > <snip>
> > 
> > I appreciate posting this, but this really should go on top of the
> > device driver work Danilo Krummrich has been doing.
> 
> If everyone agrees, I'd offer to just provide platform device / driver
> abstractions with my next patch series. This way you don't need to worry
> about aligning things with the rest of the abstractions yourself and throughout
> potential further versions of the series.

That sounds good to me, thanks!

> Just be aware that I probably won't get to work on it until after LPC.

We will not be able to review anything until after LPC either :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions
  2024-09-11 18:05         ` Ayush Singh
@ 2024-09-11 21:52           ` Danilo Krummrich
  0 siblings, 0 replies; 39+ messages in thread
From: Danilo Krummrich @ 2024-09-11 21:52 UTC (permalink / raw)
  To: Ayush Singh
  Cc: Ayush Singh, Greg Kroah-Hartman, fabien.parent, d-gole,
	lorforlinux, jkridner, robertcnelson, Andrew Davis, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Nishanth Menon, Vignesh Raghavendra,
	Tero Kristo, linux-kernel, rust-for-linux, devicetree,
	linux-arm-kernel

On Wed, Sep 11, 2024 at 11:35:43PM +0530, Ayush Singh wrote:
> On 9/11/24 23:09, Danilo Krummrich wrote:
> 
> > On 9/11/24 5:52 PM, Ayush Singh wrote:
> > > Sure, can you provide me a link to patches or maybe the branch
> > > containing that work? I also think it would be good to have the link
> > > in Zulip discussion for Platform Device and Driver.
> > 
> > Sure, please see [1]. But please be aware that I plan to rework some
> > parts
> > before sending out the next version.
> > 
> > [1] https://github.com/Rust-for-Linux/linux/tree/staging/rust-device
> 
> 
> Maybe the branch is just out of date? It still contains the generic
> structures for IdArray, IdTable and RawDeviceId.
> 
> Has something changed since the discussion here [0]?

Yes, it has; this refers to what Greg mentioned when he said that we worked
through this last weekend in his original reply.

Things will probably not remain exactly this way, but the general concept of the
abstractions will, i.e. the existance of `IdArray`, `IdTable`, and some
`DeviceId` abstraction. Please also see [1].

[1] https://lore.kernel.org/rust-for-linux/ZuHU5yrJUOKnJGrB@pollux/

> 
> 
> [0]:
> https://lore.kernel.org/rust-for-linux/2024062031-untracked-opt-3ff1@gregkh/
> 
> 
> Ayush Singh
> 

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click
  2024-09-11 20:04       ` Greg Kroah-Hartman
@ 2024-09-12  7:16         ` Ayush Singh
  2024-09-12  7:29           ` Dirk Behme
  0 siblings, 1 reply; 39+ messages in thread
From: Ayush Singh @ 2024-09-12  7:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click
  2024-09-12  7:16         ` Ayush Singh
@ 2024-09-12  7:29           ` Dirk Behme
  2024-09-12  7:39             ` Greg Kroah-Hartman
  0 siblings, 1 reply; 39+ messages in thread
From: Dirk Behme @ 2024-09-12  7:29 UTC (permalink / raw)
  To: Ayush Singh, Greg Kroah-Hartman, Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click
  2024-09-12  7:29           ` Dirk Behme
@ 2024-09-12  7:39             ` Greg Kroah-Hartman
  2024-09-12  8:17               ` Ayush Singh
  0 siblings, 1 reply; 39+ messages in thread
From: Greg Kroah-Hartman @ 2024-09-12  7:39 UTC (permalink / raw)
  To: Dirk Behme
  Cc: Ayush Singh, Ayush Singh, fabien.parent, d-gole, lorforlinux,
	jkridner, robertcnelson, Andrew Davis, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic,
	Arnd Bergmann, Nishanth Menon, Vignesh Raghavendra, Tero Kristo,
	linux-kernel, rust-for-linux, devicetree, linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click
  2024-09-12  7:39             ` Greg Kroah-Hartman
@ 2024-09-12  8:17               ` Ayush Singh
  2024-09-12  8:50                 ` Geert Stappers
  0 siblings, 1 reply; 39+ messages in thread
From: Ayush Singh @ 2024-09-12  8:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Dirk Behme
  Cc: Ayush Singh, fabien.parent, d-gole, lorforlinux, jkridner,
	robertcnelson, Andrew Davis, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic,
	Arnd Bergmann, Nishanth Menon, Vignesh Raghavendra, Tero Kristo,
	linux-kernel, rust-for-linux, devicetree, linux-arm-kernel


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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click
  2024-09-12  8:17               ` Ayush Singh
@ 2024-09-12  8:50                 ` Geert Stappers
  0 siblings, 0 replies; 39+ messages in thread
From: Geert Stappers @ 2024-09-12  8:50 UTC (permalink / raw)
  To: Ayush Singh
  Cc: Greg Kroah-Hartman, Dirk Behme, Ayush Singh, fabien.parent,
	d-gole, lorforlinux, jkridner, robertcnelson, Andrew Davis,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

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
-- 
Silence is hard to parse

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 3/8] mikrobus: Add mikrobus driver
  2024-09-11 20:03       ` Greg Kroah-Hartman
@ 2024-09-12 13:32         ` Ayush Singh
  0 siblings, 0 replies; 39+ messages in thread
From: Ayush Singh @ 2024-09-12 13:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Ayush Singh
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel

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


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/8] Add generic overlay for MikroBUS addon boards
  2024-09-11 14:27 [PATCH 0/8] Add generic overlay for MikroBUS addon boards Ayush Singh
                   ` (8 preceding siblings ...)
  2024-09-11 17:02 ` [PATCH 0/8] Add generic overlay for MikroBUS addon boards Rob Herring
@ 2024-09-13 10:05 ` Alexander Stein
  2024-09-13 10:23   ` Ayush Singh
  9 siblings, 1 reply; 39+ messages in thread
From: Alexander Stein @ 2024-09-13 10:05 UTC (permalink / raw)
  To: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Ayush Singh
  Cc: linux-kernel, rust-for-linux, devicetree, linux-arm-kernel,
	Ayush Singh

Hi,

Am Mittwoch, 11. September 2024, 16:27:17 CEST schrieb Ayush Singh:
> Hello all,
> 
> This is an attempt to add MikroBUS addon support using the approach
> described by Grove connector patch series [0].
> 
> The patch series tests out 2 addon boards + pwm and GPIO on the MikroBUS
> connector. The boards used are GPS3 Click (for UART) [1] and Weather
> Click (for I2C) [2]. Additionally, I have tested relative GPIO numbering
> using devicetree nexus nodes [3].
> 
> The patch series does not attempt to do any dynamic discovery for 1-wire
> eeprom MikroBUS addon boards, nor does it provide any sysfs entry for
> board addition/removal. The connector driver might include them after
> the basic support is ironed out, but the existing patches for dynamic
> overlays work fine.
> [sniip]

To put it in a more abstract perspective, aren't you "just" defining some
kind of connector with a fixed layout of pins and features?
It's not really different to Arduino Shields and Raspberry Pi hats, no?
Ignoring multi-purpose pins for GPIO or e.g. I2C, this is about matching
the plugin module's pins to platform-specific on-board peripherals.

Sticking the name to MikroBUS might be misleading, because this is AFAICT
just a trademark for a specific connector pin layout.
This concept could be applied for any kind of connector, where e.g.
the I2C interface is mapped to i2c0 on one platform and to lpi2c5
on a different platform.

Best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/



^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/8] Add generic overlay for MikroBUS addon boards
  2024-09-13 10:05 ` Alexander Stein
@ 2024-09-13 10:23   ` Ayush Singh
  0 siblings, 0 replies; 39+ messages in thread
From: Ayush Singh @ 2024-09-13 10:23 UTC (permalink / raw)
  To: Alexander Stein, fabien.parent, d-gole, lorforlinux, jkridner,
	robertcnelson, Andrew Davis, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Derek Kiernan, Dragan Cvetic,
	Arnd Bergmann, Greg Kroah-Hartman, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo
  Cc: linux-kernel, rust-for-linux, devicetree, linux-arm-kernel


On 9/13/24 15:35, Alexander Stein wrote:
> Hi,
>
> Am Mittwoch, 11. September 2024, 16:27:17 CEST schrieb Ayush Singh:
>> Hello all,
>>
>> This is an attempt to add MikroBUS addon support using the approach
>> described by Grove connector patch series [0].
>>
>> The patch series tests out 2 addon boards + pwm and GPIO on the MikroBUS
>> connector. The boards used are GPS3 Click (for UART) [1] and Weather
>> Click (for I2C) [2]. Additionally, I have tested relative GPIO numbering
>> using devicetree nexus nodes [3].
>>
>> The patch series does not attempt to do any dynamic discovery for 1-wire
>> eeprom MikroBUS addon boards, nor does it provide any sysfs entry for
>> board addition/removal. The connector driver might include them after
>> the basic support is ironed out, but the existing patches for dynamic
>> overlays work fine.
>> [sniip]
> To put it in a more abstract perspective, aren't you "just" defining some
> kind of connector with a fixed layout of pins and features?
> It's not really different to Arduino Shields and Raspberry Pi hats, no?
> Ignoring multi-purpose pins for GPIO or e.g. I2C, this is about matching
> the plugin module's pins to platform-specific on-board peripherals.
>
> Sticking the name to MikroBUS might be misleading, because this is AFAICT
> just a trademark for a specific connector pin layout.
> This concept could be applied for any kind of connector, where e.g.
> the I2C interface is mapped to i2c0 on one platform and to lpi2c5
> on a different platform.
>
> Best regards,
> Alexander


Yes, the only thing specific to mikroBUS in the patches is the connector 
symbols. Theoretically, it is supposed to be usable with any connector. 
MikroBUS is just the connector I am trying to apply the concept to.

I think I came across a bit too mikroBUS specific in the patches here, 
since well that is the connector I am currently trying to support, but 
as the original patches by Andrew [0] explain, this approach was 
proposed to work as a generic way to support such connectors, and even 
do connector stacking.

Trying to use it with MikroBUS shows some limitations we have with the 
current device tree stuff

1. SPI chipselect

2. Nexus nodes need the node to have some kind of existing driver

3. A good place to store the board overlays

4. Stacking needs phandle symbols table support

5. Append property support in devicetree


And of course, I might encounter more limitations as I continue to test 
more boards with it.


[0]: 
https://lore.kernel.org/linux-arm-kernel/20240702164403.29067-1-afd@ti.com/


Ayush Singh


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions
  2024-09-11 17:35     ` Danilo Krummrich
  2024-09-11 20:07       ` Greg Kroah-Hartman
@ 2024-09-14 10:11       ` Janne Grunau
  1 sibling, 0 replies; 39+ messages in thread
From: Janne Grunau @ 2024-09-14 10:11 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Greg Kroah-Hartman, Ayush Singh, fabien.parent, d-gole,
	lorforlinux, jkridner, robertcnelson, Andrew Davis, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Nishanth Menon, Vignesh Raghavendra,
	Tero Kristo, linux-kernel, rust-for-linux, devicetree,
	linux-arm-kernel, asahi

On Wed, Sep 11, 2024 at 07:35:35PM +0200, Danilo Krummrich wrote:
> On Wed, Sep 11, 2024 at 04:56:14PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 11, 2024 at 07:57:18PM +0530, Ayush Singh wrote:
> > > +/// An identifier for Platform devices.
> > > +///
> > > +/// Represents the kernel's [`struct of_device_id`]. This is used to find an appropriate
> > > +/// Platform driver.
> > > +///
> > > +/// [`struct of_device_id`]: srctree/include/linux/mod_devicetable.h
> > > +pub struct DeviceId(&'static CStr);
> > > +
> > > +impl DeviceId {
> > 
> > <snip>
> > 
> > I appreciate posting this, but this really should go on top of the
> > device driver work Danilo Krummrich has been doing.
> 
> If everyone agrees, I'd offer to just provide platform device / driver
> abstractions with my next patch series. This way you don't need to worry
> about aligning things with the rest of the abstractions yourself and throughout
> potential further versions of the series.

Covering platform device/driver abstractions in the same series would
be appreciated from asahi side. It hopefully results in earlier merge
since it avoids a dependency on the device driver abstractions.
Feel free to reach out to me for an earlier preview / rabsing of the
asahi driver.
https://github.com/AsahiLinux/linux/tree/bits/210-gpu has all relevant
rust changes from 6.11-rc but I plan to rebase onto 6.11 in the next
days and possibly import changes from 6.12-rc* / rust-next.

Thanks
Janne

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 0/8] Add generic overlay for MikroBUS addon boards
  2024-09-11 17:02 ` [PATCH 0/8] Add generic overlay for MikroBUS addon boards Rob Herring
@ 2024-09-20 16:51   ` Ayush Singh
  0 siblings, 0 replies; 39+ messages in thread
From: Ayush Singh @ 2024-09-20 16:51 UTC (permalink / raw)
  To: Rob Herring
  Cc: fabien.parent, d-gole, lorforlinux, jkridner, robertcnelson,
	Andrew Davis, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Krzysztof Kozlowski, Conor Dooley, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, Nishanth Menon,
	Vignesh Raghavendra, Tero Kristo, linux-kernel, rust-for-linux,
	devicetree, linux-arm-kernel


On 9/11/24 22:32, Rob Herring wrote:
> On Wed, Sep 11, 2024 at 07:57:17PM +0530, Ayush Singh wrote:
>> Hello all,
>>
>> This is an attempt to add MikroBUS addon support using the approach
>> described by Grove connector patch series [0].
>>
>> The patch series tests out 2 addon boards + pwm and GPIO on the MikroBUS
>> connector. The boards used are GPS3 Click (for UART) [1] and Weather
>> Click (for I2C) [2]. Additionally, I have tested relative GPIO numbering
>> using devicetree nexus nodes [3].
>>
>> The patch series does not attempt to do any dynamic discovery for 1-wire
>> eeprom MikroBUS addon boards, nor does it provide any sysfs entry for
>> board addition/removal. The connector driver might include them after
>> the basic support is ironed out, but the existing patches for dynamic
>> overlays work fine.
>>
>> The patch series has been tested on BeaglePlay [4].
>>
>> I will now go over individual parts of the patch series, but for a
>> better picture of the approach, it would be best to checkout [0] first.
>>
>> MikroBUS connector driver
>> -------------------------
>>
>> Just a stub platform driver for now. Will be extended in the future for
>> dynamic board discovery using 1-wire eeprom present in some MikroBUS
>> addon boards.
>>
>> While it is a stub driver, disabling it will make the GPIO connector
>> nexus node unreachable (any driver attempting to use it will enter
>> differed probing). So it is required.
>>
>> MikroBUS connector Devicetree
>> ------------------------------
>>
>> The connector devicetree defines the MikroBUS GPIO nexus node. This
>> allows using pin numbering relative to MikroBUS connector pins in the
>> addon boards overlay. Currently, the patch uses a clockwise numbering:
>>
>>    0: PWM
>>    1: INT
>>    2: RX
>>    3: TX
>>    4: SCL
>>    5: SDA
>>    6: MOSI
>>    7: MISO
>>    8: SCK
>>    9: CS
>>    10: RST
>>    11: AN
>>
>> Additionally, in case PWM pin is not using channel 0, a nexus node for pwm
>> should also be used and go in the connector devicetree.
>>
>> MikroBUS connector symbols overlay
>> ----------------------------------
>>
>> To make an overlay generic we need a standard name scheme which we
>> use across base boards. For the connector pins the pinmux phandle
>> shall be:
>>
>> <connector-name>_<pin-name>_mux_<pin-function>
>>
>> For the parent provider phandle, we use a similar naming scheme:
>>
>> <connector-name>_<pin-name>_<pin-function>
>>
>> For GPIO controller, I am using `MIKROBUS_GPIO` name since with nexus
>> nodes, we do not need to define individual pin gpio controllers.
>>
>> The string symbols can be replaced with phandles once [5] is accepted.
>> That will make connector stacking much simpler.
>>
>> MikroBUS addon board overlay
>> ----------------------------
>>
>> The patch puts the addon board overlays in their own directory. I am
>> using the following naming scheme for MikroBUS addon boards:
>>
>> <vendor>-<product_id>.dtso
>>
>> Mikroe [6] lists this for all boards in their website, but I am not sure
>> if other vendors have a product_id.
>>
>> This naming also makes future dynamic discovery easier, since click id
>> spec [7] contains vendor_id and product_id in the header.
>>
>> Usage
>> -----
>>
>> So what does this all look like? Let's take an example of a BeaglePlay
>> with one MikroBUS connectors for which we have physically attached a
>> Wather click module to the first connector. Doing ahead of time
>> command-line DT overlay application:
>>
>> ./fdtoverlay \
>> 	-o output.dtb \
>> 	-i k3-am625-beagleplay.dtb \
>> 		k3-am625-beagleplay-mikrobus-connector0.dtbo mikroe-5761.dtbo
>>
>> Open Items
>> ----------
>>
>> - SPI Support:
>>    Currently SPI dt requires `reg` property to specify the
>>    chipselect to use. This, makes the SPI device overlay dependent on the
>>    SPI controller. Thus for SPI support, we need a way to allow defining
>>    SPI chipselect relative to MikroBUS pins, and not the actual device
>>    controller.
>>
>>    One possible solution is to introduce something like `named-reg` and
>>    allow selecting the chipselect by string array. But this probably
>>    requires more discussion so I have left out SPI support for now.
>>
>>    NOTE: pins other than the CS MikroBUS pin can be used as chipselect.
>>    See [8].
>>
>> - Controller symbol duplication
>>    The current symbol scheme has multiple symbols for the same underlying
>>    controller (Eg: MIKROBUS_SCL_MUX_I2C_SCL and MIKROBUS_SDA_MUX_I2C_SDA)
>>    point to the same i2c controller.
>>
>>    I think both of them will always use the same i2c controller, but
>>    maybe there are some exceptions? So I have left it as it is for this
>>    patch series. The same thing also applies to UART and SPI.
>>
>>    NOTE: with the use of nexus node for GPIO, the GPIO controller symbol
>>    will be the same for all pins.
>>
>> - Nexus node dt-bindings
>>    I am not quite sure how to deal with the nexus node properties
>>    (#gpio-cells, gpio-map, gpio-map-mask, gpio-map-pass-thru) since they
>>    seem to conflict with upstream gpio schema (gpio-controller is a
>>    dependency of #gpio-cells).
> Please submit a fix to dtschema. Either GH PR or patch to
> devicetree-spec list.
>
> Rob


I have created a GH PR [0].


[0]: https://github.com/devicetree-org/dt-schema/pull/143


Ayush Singh


^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2024-09-20 16:51 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-11 14:27 [PATCH 0/8] Add generic overlay for MikroBUS addon boards Ayush Singh
2024-09-11 14:27 ` [PATCH 1/8] rust: kernel: Add Platform device and driver abstractions Ayush Singh
2024-09-11 14:56   ` Greg Kroah-Hartman
2024-09-11 15:52     ` Ayush Singh
2024-09-11 17:39       ` Danilo Krummrich
2024-09-11 18:05         ` Ayush Singh
2024-09-11 21:52           ` Danilo Krummrich
2024-09-11 17:35     ` Danilo Krummrich
2024-09-11 20:07       ` Greg Kroah-Hartman
2024-09-14 10:11       ` Janne Grunau
2024-09-11 14:27 ` [PATCH 2/8] dt-bindings: connector: Add MikorBUS connector Ayush Singh
2024-09-11 15:26   ` Rob Herring (Arm)
2024-09-11 17:26   ` Rob Herring
2024-09-11 18:00     ` Ayush Singh
2024-09-11 14:27 ` [PATCH 3/8] mikrobus: Add mikrobus driver Ayush Singh
2024-09-11 14:57   ` Greg Kroah-Hartman
2024-09-11 16:02     ` Ayush Singh
2024-09-11 20:03       ` Greg Kroah-Hartman
2024-09-12 13:32         ` Ayush Singh
2024-09-11 15:48   ` Andrew Lunn
2024-09-11 14:27 ` [PATCH 4/8] dts: ti: k3-am625-beagleplay: Enable mikroBUS connector Ayush Singh
2024-09-11 14:27 ` [PATCH 5/8] dts: ti: beagleplay: Add mikrobus connector symbols Ayush Singh
2024-09-11 14:27 ` [PATCH 6/8] addon_boards: Add addon_boards plumbing Ayush Singh
2024-09-11 15:00   ` Greg Kroah-Hartman
2024-09-11 16:09     ` Ayush Singh
2024-09-11 14:27 ` [PATCH 7/8] addon_boards: mikrobus: Add Weather Click Ayush Singh
2024-09-11 14:27 ` [PATCH 8/8] addon_boards: mikrobus: Add GPS3 Click Ayush Singh
2024-09-11 14:58   ` Greg Kroah-Hartman
2024-09-11 15:56     ` Ayush Singh
2024-09-11 20:04       ` Greg Kroah-Hartman
2024-09-12  7:16         ` Ayush Singh
2024-09-12  7:29           ` Dirk Behme
2024-09-12  7:39             ` Greg Kroah-Hartman
2024-09-12  8:17               ` Ayush Singh
2024-09-12  8:50                 ` Geert Stappers
2024-09-11 17:02 ` [PATCH 0/8] Add generic overlay for MikroBUS addon boards Rob Herring
2024-09-20 16:51   ` Ayush Singh
2024-09-13 10:05 ` Alexander Stein
2024-09-13 10:23   ` Ayush Singh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).