On Mon, Apr 25, 2022 at 5:16 PM Rob Herring <robh@xxxxxxxxxx> wrote: > > On Mon, Apr 25, 2022 at 02:16:02PM -0500, Daniel Kaehn wrote: > > Adds dt-binding for snd-serial-generic serial MIDI driver > > Bindings are for h/w and there's no such thing as generic h/w. There are > some exceptions but you'll have to justify why this is special. > Thanks for taking the time to look at this! Not entirely sure if you mean that I'll need to justify the existence / need for this binding, or the use of the term 'generic' -- just in case, I'll make sure to respond to both. Note that I'm justifying my reasoning for submitting the binding - but I'm uncertain myself if my reasoning is valid, as someone new to kernel development. The intent of this binding is to signify that a serial port (namely a UART) is connected in hardware to a MIDI decoupling circuit, which then connects to some (any) sort of MIDI device, either directly to an on-board device, or via a jack/connector. In a sense, the MIDI device that this connects to is 'generic', as the identity of the device does not need to be known to interface with it over MIDI for most use cases. I see how this is a bit of an oddball, since it's not specifically describing a particular hardware device attached to a UART (like some of the bluetooth modules are), but thought this sort of binding might be permissible because of things like the gpio-matrix-keypad binding, which doesn't describe specific switches, just how thoise switches are wired, and what GPIO they use, which is all that is needed to interface with them. Some MIDI devices implement extra low-level features like device multiplexing, but these aren't (to my knowledge) common, and are beyond what this supports. The reason that the corresponding driver written has the name 'generic' is for an entirely different reason. A "serial MIDI" driver already exists in the kernel, however, it interfaces only with u16550-compatible UARTs. This driver uses the serial bus, making it work with 'generic' serial devices. If this comment was directed toward the use of 'generic' in the commit message and binding descriptions: I can reword them to be more specific and to avoid the term. > > > Signed-off-by: Daniel Kaehn <kaehndan@xxxxxxxxx> > > --- > > .../devicetree/bindings/sound/serialmidi.yaml | 41 +++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/sound/serialmidi.yaml > > > > diff --git a/Documentation/devicetree/bindings/sound/serialmidi.yaml b/Documentation/devicetree/bindings/sound/serialmidi.yaml > > new file mode 100644 > > index 000000000000..38ef49a0c2f9 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/serialmidi.yaml > > @@ -0,0 +1,41 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > + > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/sound/serialmidi.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Generic Serial MIDI Device > > + > > +maintainers: > > + - Daniel Kaehn <kaehndan@xxxxxxxxx> > > + > > +description: | > > Don't need '|' unless there is formatting to preserve. > Will fix. > > + Generic MIDI interface using a serial device. Can only be set to use standard speeds > > + corresponding to supported baud rates of the underlying serial device. If standard MIDI > > + speed of 31.25 kBaud is needed, configure the clocks of the underlying serial device > > + so that a requested speed of 38.4 kBaud resuts in the standard MIDI baud rate. > > + > > +properties: > > + compatible: > > + const: serialmidi > > + > > + speed: > > Not a standard property and we already have 2 of them concerning baud > rate. > Thanks for the correction - I'll switch this to the existing "baud" property. I somehow missed that there was a fixed list of standard properties to be used, and just chose 'speed' as opposed to 'current-speed' which I saw on serial bindings, since this speed is fixed and can't (and shouldn't need to be) changed. "baud" describes this well enough. > > + maxItems: 1 > > + description: | > > + Speed to set the serial port to when the MIDI device is opened. > > + If not specified, the underlying serial device is allowed to use its configured default speed. > > + > > +required: > > + - compatible > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + serial { > > + midi { > > + compatible = "serialmidi"; > > + speed = <38400>; > > + }; > > + }; > > -- > > 2.33.0 > > > > Thanks, Daneil Kaehn
