Re: [PATCH 1/3] dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA

[Jonathan Cameron]

On Thu,  7 May 2026 12:47:22 +0000
Vladislav Kulikov <vlad.kulikov.c@xxxxxxxxx> wrote:

> Add device tree binding documentation for the MEMSIC MMC5983MA
> 3-axis magnetometer connected via I2C.

HI Vladislav,

Usual question to answer.  How is this different from existing memsic
magnetometers that are supported?  Even if the driver is different
is there a reason to have a separate binding?  Note that same question
will apply to the driver - just provide some brief notes on how it
is different enough from existing devices. 

The others are all in trivial bindings.  They maybe should not be!

A few other comments inline.

Whilst you comment you've left the SPI side out, it would be good to still
have a DT binding even if the driver doesn't support it.


> 
> Signed-off-by: Vladislav Kulikov <vlad.kulikov.c@xxxxxxxxx>
> ---
>  .../iio/magnetometer/memsic,mmc5983.yaml      | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml b/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
> new file mode 100644
> index 000000000000..bbe2aa597f75
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/magnetometer/memsic,mmc5983.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MEMSIC MMC5983MA 3-axis magnetic sensor
> +
> +maintainers:
> +  - Vladislav Kulikov <vlad.kulikov.c@xxxxxxxxx>
> +
> +properties:
> +  compatible:
> +    const: memsic,mmc5983
> +
> +  reg:
> +    maxItems: 1
Binding should try to be as complete as possible, even if the driver
doesn't yet use some features.  Looks like we have an interrupt to 
describe.  Also vddio is missing based on the datasheet google found for me:

https://media.digikey.com/pdf/Data%20Sheets/MEMSIC%20PDFs/MMC5983MA_RevA_4-3-19.pdf

> +
> +  vdd-supply:
> +    description: Regulator that provides power to the sensor
> +
> +required:
> +  - compatible
> +  - reg
I'm guessing it doesn't work well without a power supply
   - vdd-supply 
should be here.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        magnetometer@30 {
> +            compatible = "memsic,mmc5983";
> +            reg = <0x30>;
> +            vdd-supply = <&vdd_3v3_reg>;
> +        };
> +    };