Re: [PATCH 05/14] dt-bindings: trivial-devices: add GOcontroll Moduline IO modules

[Rob Herring <robh@kernel.org>]

On Tue, Feb 25, 2025 at 12:24:09PM +0000, Maud Spierings | GOcontroll wrote:
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Tuesday, February 25, 2025 12:52 PM
>  
> >On Tue, Feb 25, 2025 at 07:39:52AM +0000, Maud Spierings | GOcontroll wrote:
> >> From: Rob Herring <robh@kernel.org>
> >> Sent: Monday, February 24, 2025 9:44 PM
> >>  
> >> >On Mon, Feb 24, 2025 at 02:50:55PM +0100, Maud Spierings wrote:
> >> >> The main point of the Moduline series of embedded controllers is its
> >> >> ecosystem of IO modules, these currently are operated through the spidev
> >> >> interface. Ideally there will be a full dedicated driver in the future.
> >> >>
> >> >> Add the gocontroll moduline-module-slot device to enable the required
> >> >> spidev interface.
> >> >>
> >> >> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
> >> >> ---
> >> >>  Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
> >> >>  1 file changed, 2 insertions(+)
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> >> >> index 8255bb590c0cc619d15b27dcbfd3aa85389c0a54..24ba810f91b73efdc615c7fb46f771a300926f05 100644
> >> >> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> >> >> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> >> >> @@ -107,6 +107,8 @@ properties:
> >> >>            - fsl,mpl3115
> >> >>              # MPR121: Proximity Capacitive Touch Sensor Controller
> >> >>            - fsl,mpr121
> >> >> +            # GOcontroll Moduline module slot for spi based IO modules
> >> >
> >> >I couldn't find anything about SPI for GOcontroll Moduline. Can you
> >> >point me to what this hardware looks like. Based on what I did find,
> >> >this seems incomplete and not likely a trivial device.
> >>
> >> I'll give some more details, if there is a v2 of this patch I will also
> >> add more information in the commit message.
> >>
> >> The module slots have a number of pins, a lot of them currently unused as
> >> they have not found a function yet, this is very much still a developing
> >> product. The currently used interfaces to the SoC are:
> >> 1. SPI bus as a spidev to ease developing new modules and quickly
> >> integrate them. This is the main communication interface for control and
> >> firmware updates.
> >> 2. A reset pin, this is/was driven with the gpio-led driver but I doubt
> >> that would get accepted upstream so I intend to switch to the much better
> >> suited libgpio.
> >
> >reset-gpios is not in trivial devices, so that's already a hint you
> >cannot use this binding.
> >
> >> 3. An interrupt pin, this is currently only used in the firmware update
> >> utility [2] to speed up the update process. Other communication is done at
> >> a regular interval.
> >>
> >> What is unused:
> >> 1. A potentially multi-master i2c bus between all the module slots and
> >> the SoC
> >> 2. An SMBus alert line is shared between the modules, but not the SoC.
> >> 3. A shared line designated as a clock line, intended to in the future
> >> aid with synchronizing modules to each other for time critical control.
> >>
> >> current software that is used to work with the modules can be found at
> >> [2] and [3], one of them is a Node-RED module the other is a blockset for
> >> Matlab/Simulink generated code.
> >>
> >> If you know a better way I could describe this in the devicetree then I
> >
> >You need dedicated binding where you describe entire device, entire
> >hardware, not what your driver supports in current release.
> 
> I see now that I also forgot the patch that adds this compatible to the
> spidev driver. Didn't check for the spidevs in testing I guess.
> 
> Could I write bindings for this device, and then add the compatible to the
> spidev driver for now? So it probes that driver, and then later when there
> is a driver remove the compatible there and keep it only in the purpose
> built driver?
> 
> So I'll write gocontroll,moduline-module-slot.yaml, don't quite know where
> that would go. Define all these attributes in there and then add the 
> compatible to drivers/spi/spidev.c
> 
> Is that okay?

Yes. Bindings are forever, but drivers change. ;)

Perhaps put it in connector/ as this looks a bit like a connector. Do 
you envision DT overlays for the IO modules? Or modules don't have 
sub-devices you need to describe? There's some effort to on connector 
bindings (for mikrobus in particular) in order to de-couple host 
buses/signals from the modules (i.e. so a DT overlay can be applied to 
any DT defining the connector).

Rob