Hello,
This series add OF device ID tables to mfd I2C drivers whose devices are
either used in Device Tree source files or are listed in binding docs as
a compatible string.
That's done because the plan is to change the I2C core to report proper OF
modaliases instead of always reporting a MODALIAS=i2c:<foo> regardless if
a device was registered via DT or using the legacy platform data mechanism.
So these patches will make sure that mfd I2C drivers modules will continue
to be autoloaded once the I2C core is changed to report proper OF modalias.
Users didn't have a vendor prefix in the used compatible strings, but since
there wasn't a DT binding document for these drivers, it can be said that
were working for mere luck and so this series fixes the users and add a DT
binding doc for the drivers.
It's safe to apply the patches independently.
Best regards,
Javier
Changes in v3:
- Add a vendor prefix to the compatible string (Rob Herring).
- Add a vendor prefix to the compatible string (Rob Herring).
Changes in v2:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
Javier Martinez Canillas (7):
mfd: Add Device Tree bindings document for retu/tahvo ASIC chips
mfd: retu: Add OF device ID table
i2c: i2c-cbus-gpio: Add vendor prefix to retu node in example
ARM: dts: n8x0: Add vendor prefix to retu node
mfd: Add Device Tree bindings document for TI tps6105x chip
mfd: tps6105x: Add OF device ID table
ARM: ux500: Add vendor prefix to tps61052 node
.../devicetree/bindings/i2c/i2c-cbus-gpio.txt | 2 +-
Documentation/devicetree/bindings/mfd/retu.txt | 19 +++++++++++++++++++
Documentation/devicetree/bindings/mfd/tps6105x.txt | 17 +++++++++++++++++
arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 2 +-
arch/arm/boot/dts/ste-hrefprev60.dtsi | 2 +-
drivers/mfd/retu-mfd.c | 8 ++++++++
drivers/mfd/tps6105x.c | 8 ++++++++
7 files changed, 55 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/retu.txt
create mode 100644 Documentation/devicetree/bindings/mfd/tps6105x.txt
--
2.9.3
There are Device Tree source files defining a device node for the
retu/tahvo I2C chip, but there isn't a DT binding document for it.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
Changes in v3: None
Changes in v2: None
Documentation/devicetree/bindings/mfd/retu.txt | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/retu.txt
diff --git a/Documentation/devicetree/bindings/mfd/retu.txt b/Documentation/devicetree/bindings/mfd/retu.txt
new file mode 100644
index 000000000000..2309e599a731
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/retu.txt
@@ -0,0 +1,19 @@
+* Device tree bindings for Nokia Retu and Tahvo multi-function device
+
+Retu and Tahvo are a multi-function devices found on Nokia Internet
+Tablets (770, N800 and N810).
+
+Required properties:
+- compatible: "nokia,retu-mfd" or "nokia,tahvo-mfd"
+- reg: Specifies the I2C slave address of the ASIC chip
+
+Example:
+
+i2c0 {
+ retu_mfd: retu@1 {
+ compatible = "nokia,retu-mfd";
+ interrupt-parent = <&gpio4>;
+ interrupts = <12 IRQ_TYPE_EDGE_RISING>;
+ reg = <0x1>;
+ };
+};
--
2.9.3
The example contains a device node for a retu-mfd device,
but its compatible string doesn't have a vendor prefix.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
Changes in v3: None
Changes in v2: None
Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt b/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt
index 8ce9cd2855b5..32bcb1943f00 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-cbus-gpio.txt
@@ -21,7 +21,7 @@ i2c@0 {
#size-cells = <0>;
retu-mfd: retu@1 {
- compatible = "retu-mfd";
+ compatible = "nokia,retu-mfd";
reg = <0x1>;
};
};
--
2.9.3
There are Device Tree source files defining a device node for the
tps61050/61052 I2C chip but there isn't a binding document for it.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
Changes in v3: None
Changes in v2: None
Documentation/devicetree/bindings/mfd/tps6105x.txt | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/tps6105x.txt
diff --git a/Documentation/devicetree/bindings/mfd/tps6105x.txt b/Documentation/devicetree/bindings/mfd/tps6105x.txt
new file mode 100644
index 000000000000..c076f28575fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/tps6105x.txt
@@ -0,0 +1,17 @@
+* Device tree bindings for TI TPS61050/61052 Boost Converters
+
+The TP61050/TPS61052 is a high-power "white LED driver". This boost converter
+is also used for other things than white LEDs, and also contains a GPIO pin.
+
+Required properties:
+- compatible: "ti,tps61050" or "ti,tps61052"
+- reg: Specifies the I2C slave address
+
+Example:
+
+i2c0 {
+ tps61052@33 {
+ compatible = "ti,tps61052";
+ reg = <0x33>;
+ };
+};
--
2.9.3
The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:<device>.
But this could change in the future so the correct approach is to have a
OF device ID table if the devices are registered via OF.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
Changes in v3:
- Add a vendor prefix to the compatible string (Rob Herring).
Changes in v2:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
drivers/mfd/tps6105x.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mfd/tps6105x.c b/drivers/mfd/tps6105x.c
index baa12ea666fb..187848c93779 100644
--- a/drivers/mfd/tps6105x.c
+++ b/drivers/mfd/tps6105x.c
@@ -173,9 +173,17 @@ static const struct i2c_device_id tps6105x_id[] = {
};
MODULE_DEVICE_TABLE(i2c, tps6105x_id);
+static const struct of_device_id tps6105x_of_match[] = {
+ { .compatible = "ti,tps61050" },
+ { .compatible = "ti,tps61052" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, tps6105x_of_match);
+
static struct i2c_driver tps6105x_driver = {
.driver = {
.name = "tps6105x",
+ .of_match_table = tps6105x_of_match,
},
.probe = tps6105x_probe,
.remove = tps6105x_remove,
--
2.9.3
The tps61052 device node doesn't have a vendor prefix
in its compatible string, fix it by adding one.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
Changes in v3: None
Changes in v2: None
arch/arm/boot/dts/ste-hrefprev60.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/ste-hrefprev60.dtsi b/arch/arm/boot/dts/ste-hrefprev60.dtsi
index 5882a2606ac3..3f14b4df69b4 100644
--- a/arch/arm/boot/dts/ste-hrefprev60.dtsi
+++ b/arch/arm/boot/dts/ste-hrefprev60.dtsi
@@ -30,7 +30,7 @@
i2c@80004000 {
tps61052@33 {
- compatible = "tps61052";
+ compatible = "ti,tps61052";
reg = <0x33>;
};
--
2.9.3
The retu-mfd device node doesn't have a vendor prefix
in its compatible string, fix it by adding one.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
Changes in v3: None
Changes in v2: None
arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
index 7e5ffc583c90..a633a55820bb 100644
--- a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
+++ b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
@@ -16,7 +16,7 @@
#address-cells = <1>;
#size-cells = <0>;
retu_mfd: retu@1 {
- compatible = "retu-mfd";
+ compatible = "nokia,retu-mfd";
interrupt-parent = <&gpio4>;
interrupts = <12 IRQ_TYPE_EDGE_RISING>;
reg = <0x1>;
--
2.9.3
The driver doesn't have a struct of_device_id table but supported devices
are registered via Device Trees. This is working on the assumption that a
I2C device registered via OF will always match a legacy I2C device ID and
that the MODALIAS reported will always be of the form i2c:<device>.
But this could change in the future so the correct approach is to have a
OF device ID table if the devices are registered via OF.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
Changes in v3:
- Add a vendor prefix to the compatible string (Rob Herring).
Changes in v2:
- Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
drivers/mfd/retu-mfd.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/mfd/retu-mfd.c b/drivers/mfd/retu-mfd.c
index d4c114abeb75..937a1c21eec4 100644
--- a/drivers/mfd/retu-mfd.c
+++ b/drivers/mfd/retu-mfd.c
@@ -308,9 +308,17 @@ static const struct i2c_device_id retu_id[] = {
};
MODULE_DEVICE_TABLE(i2c, retu_id);
+static const struct of_device_id retu_of_match[] = {
+ { .compatible = "nokia,retu-mfd" },
+ { .compatible = "nokia,tahvo-mfd" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, retu_of_match);
+
static struct i2c_driver retu_driver = {
.driver = {
.name = "retu-mfd",
+ .of_match_table = retu_of_match,
},
.probe = retu_probe,
.remove = retu_remove,
--
2.9.3
On Sat, 01 Apr 2017, Javier Martinez Canillas wrote:
> There are Device Tree source files defining a device node for the
> retu/tahvo I2C chip, but there isn't a DT binding document for it.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
> Documentation/devicetree/bindings/mfd/retu.txt | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/retu.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/retu.txt b/Documentation/devicetree/bindings/mfd/retu.txt
> new file mode 100644
> index 000000000000..2309e599a731
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/retu.txt
> @@ -0,0 +1,19 @@
> +* Device tree bindings for Nokia Retu and Tahvo multi-function device
> +
> +Retu and Tahvo are a multi-function devices found on Nokia Internet
> +Tablets (770, N800 and N810).
More information please. What functions to they serve?
> +Required properties:
> +- compatible: "nokia,retu-mfd" or "nokia,tahvo-mfd"
> +- reg: Specifies the I2C slave address of the ASIC chip
> +
> +Example:
> +
> +i2c0 {
> + retu_mfd: retu@1 {
Please avoid the term MFD at all times when using Device Tree. DT is
meant to be OS agnostic and MFD is a Linuxisum.
> + compatible = "nokia,retu-mfd";
> + interrupt-parent = <&gpio4>;
> + interrupts = <12 IRQ_TYPE_EDGE_RISING>;
> + reg = <0x1>;
> + };
> +};
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Sat, 01 Apr 2017, Javier Martinez Canillas wrote:
> The driver doesn't have a struct of_device_id table but supported devices
> are registered via Device Trees. This is working on the assumption that a
> I2C device registered via OF will always match a legacy I2C device ID and
> that the MODALIAS reported will always be of the form i2c:<device>.
>
> But this could change in the future so the correct approach is to have a
> OF device ID table if the devices are registered via OF.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
>
> ---
>
> Changes in v3:
> - Add a vendor prefix to the compatible string (Rob Herring).
>
> Changes in v2:
> - Don't use of_match_ptr() to avoid build warning when CONFIG_OF is disabled.
>
> drivers/mfd/retu-mfd.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mfd/retu-mfd.c b/drivers/mfd/retu-mfd.c
> index d4c114abeb75..937a1c21eec4 100644
> --- a/drivers/mfd/retu-mfd.c
> +++ b/drivers/mfd/retu-mfd.c
> @@ -308,9 +308,17 @@ static const struct i2c_device_id retu_id[] = {
> };
> MODULE_DEVICE_TABLE(i2c, retu_id);
>
> +static const struct of_device_id retu_of_match[] = {
> + { .compatible = "nokia,retu-mfd" },
> + { .compatible = "nokia,tahvo-mfd" },
Please drop the "-mfd".
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, retu_of_match);
> +
> static struct i2c_driver retu_driver = {
> .driver = {
> .name = "retu-mfd",
> + .of_match_table = retu_of_match,
> },
> .probe = retu_probe,
> .remove = retu_remove,
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Sat, 01 Apr 2017, Javier Martinez Canillas wrote:
> There are Device Tree source files defining a device node for the
> tps61050/61052 I2C chip but there isn't a binding document for it.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
> Documentation/devicetree/bindings/mfd/tps6105x.txt | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/tps6105x.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/tps6105x.txt b/Documentation/devicetree/bindings/mfd/tps6105x.txt
> new file mode 100644
> index 000000000000..c076f28575fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/tps6105x.txt
> @@ -0,0 +1,17 @@
> +* Device tree bindings for TI TPS61050/61052 Boost Converters
> +
> +The TP61050/TPS61052 is a high-power "white LED driver". This boost converter
> +is also used for other things than white LEDs, and also contains a GPIO pin.
What functions does it offer?
> +Required properties:
> +- compatible: "ti,tps61050" or "ti,tps61052"
> +- reg: Specifies the I2C slave address
> +
> +Example:
> +
> +i2c0 {
> + tps61052@33 {
> + compatible = "ti,tps61052";
> + reg = <0x33>;
> + };
> +};
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Hello Lee,
Thanks a lot for your feedback.
On 04/03/2017 07:13 AM, Lee Jones wrote:
> On Sat, 01 Apr 2017, Javier Martinez Canillas wrote:
>
>> There are Device Tree source files defining a device node for the
>> retu/tahvo I2C chip, but there isn't a DT binding document for it.
>>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>> ---
>>
>> Changes in v3: None
>> Changes in v2: None
>>
>> Documentation/devicetree/bindings/mfd/retu.txt | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/retu.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/retu.txt b/Documentation/devicetree/bindings/mfd/retu.txt
>> new file mode 100644
>> index 000000000000..2309e599a731
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/retu.txt
>> @@ -0,0 +1,19 @@
>> +* Device tree bindings for Nokia Retu and Tahvo multi-function device
>> +
>> +Retu and Tahvo are a multi-function devices found on Nokia Internet
>> +Tablets (770, N800 and N810).
>
> More information please. What functions to they serve?
>
I'm not really familiar with these drivers, I'm just doing this as a part
of the bigger work to fix the I2C core to report a proper OF modaliases.
So I took the information I could find in the driver Kconfig and headers,
but I'll see if I can find more information in the driver or if there is
documentation about these online.
>> +Required properties:
>> +- compatible: "nokia,retu-mfd" or "nokia,tahvo-mfd"
>> +- reg: Specifies the I2C slave address of the ASIC chip
>> +
>> +Example:
>> +
>> +i2c0 {
>> + retu_mfd: retu@1 {
>
> Please avoid the term MFD at all times when using Device Tree. DT is
> meant to be OS agnostic and MFD is a Linuxisum.
>
Yes, sorry. I just copied the DT snippet from a DTS that used this, I'll
remove it on the next version.
>> + compatible = "nokia,retu-mfd";
>> + interrupt-parent = <&gpio4>;
>> + interrupts = <12 IRQ_TYPE_EDGE_RISING>;
>> + reg = <0x1>;
>> + };
>> +};
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
Hello Lee,
On 04/03/2017 07:15 AM, Lee Jones wrote:
[snip]
>>
>> +static const struct of_device_id retu_of_match[] = {
>> + { .compatible = "nokia,retu-mfd" },
>> + { .compatible = "nokia,tahvo-mfd" },
>
> Please drop the "-mfd".
>
Yes, I also didn't like it but I didn't want to change it since that would
mean that backward compatiblity and bisect-ability will be broken by this
change.
In other words, just adding a vendor prefix won't cause an issue if patches
are merged independently since if DTS patches are merged before, the driver
will still lookup using the I2C device ID table. And if the drivers patches
are picked before, the DTS will match using the OF device ID table.
But changing to "nokia,retu" and "nokia,tahvo" means that you will need to
pick all patches and also that the DTS and drivers changes will have to be
done in the same patch. If you are OK with that, then I can change in the
next version.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
Hello Lee,
On 04/03/2017 07:15 AM, Lee Jones wrote:
[snip]
>> +
>> +The TP61050/TPS61052 is a high-power "white LED driver". This boost converter
>> +is also used for other things than white LEDs, and also contains a GPIO pin.
>
> What functions does it offer?
>
Same comment than before, I'm not really familiar with this driver. But I'll
look what are the MFD cell instantiated to better understand dev functions
and expand this documentation accordingly.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
On Mon, Apr 03, 2017 at 11:45:14AM -0400, Javier Martinez Canillas wrote:
> Hello Lee,
>
> On 04/03/2017 07:15 AM, Lee Jones wrote:
>
> [snip]
>
> >>
> >> +static const struct of_device_id retu_of_match[] = {
> >> + { .compatible = "nokia,retu-mfd" },
> >> + { .compatible = "nokia,tahvo-mfd" },
> >
> > Please drop the "-mfd".
> >
>
> Yes, I also didn't like it but I didn't want to change it since that would
> mean that backward compatiblity and bisect-ability will be broken by this
> change.
>
> In other words, just adding a vendor prefix won't cause an issue if patches
> are merged independently since if DTS patches are merged before, the driver
> will still lookup using the I2C device ID table. And if the drivers patches
> are picked before, the DTS will match using the OF device ID table.
>
> But changing to "nokia,retu" and "nokia,tahvo" means that you will need to
> pick all patches and also that the DTS and drivers changes will have to be
> done in the same patch. If you are OK with that, then I can change in the
> next version.
tahvo is not documented nor used in any dts (in the kernel at least).
retu is used by 1 board and happened to work, but was never documented.
So I think it is okay to change unless the N800 folks object.
Rob
[adding OMAP folks to cc list]
Hello Rob,
On 04/03/2017 06:20 PM, Rob Herring wrote:
> On Mon, Apr 03, 2017 at 11:45:14AM -0400, Javier Martinez Canillas wrote:
>> Hello Lee,
>>
>> On 04/03/2017 07:15 AM, Lee Jones wrote:
>>
>> [snip]
>>
>>>>
>>>> +static const struct of_device_id retu_of_match[] = {
>>>> + { .compatible = "nokia,retu-mfd" },
>>>> + { .compatible = "nokia,tahvo-mfd" },
>>>
>>> Please drop the "-mfd".
>>>
>>
>> Yes, I also didn't like it but I didn't want to change it since that would
>> mean that backward compatiblity and bisect-ability will be broken by this
>> change.
>>
>> In other words, just adding a vendor prefix won't cause an issue if patches
>> are merged independently since if DTS patches are merged before, the driver
>> will still lookup using the I2C device ID table. And if the drivers patches
>> are picked before, the DTS will match using the OF device ID table.
>>
>> But changing to "nokia,retu" and "nokia,tahvo" means that you will need to
>> pick all patches and also that the DTS and drivers changes will have to be
>> done in the same patch. If you are OK with that, then I can change in the
>> next version.
>
> tahvo is not documented nor used in any dts (in the kernel at least).
> retu is used by 1 board and happened to work, but was never documented.
> So I think it is okay to change unless the N800 folks object.
>
I'm fine with changing it (in fact I just want to fix the I2C of modalias
reporting). Does this mean that backward compatibility and bisect-ability
should be preserved? Or it's OK to split the changes in different patches?
> Rob
>
Best regards,
---
Javier Martinez Canillas
Open Source Group
Samsung Research America
* Aaro Koskinen <[email protected]> [170403 15:56]:
> Hi,
>
> On Mon, Apr 03, 2017 at 06:24:39PM -0400, Javier Martinez Canillas wrote:
> > On 04/03/2017 06:20 PM, Rob Herring wrote:
> > > On Mon, Apr 03, 2017 at 11:45:14AM -0400, Javier Martinez Canillas wrote:
> > >> Hello Lee,
> > >>
> > >> On 04/03/2017 07:15 AM, Lee Jones wrote:
> > >>
> > >> [snip]
> > >>
> > >>>>
> > >>>> +static const struct of_device_id retu_of_match[] = {
> > >>>> + { .compatible = "nokia,retu-mfd" },
> > >>>> + { .compatible = "nokia,tahvo-mfd" },
> > >>>
> > >>> Please drop the "-mfd".
> > >>>
> > >>
> > >> Yes, I also didn't like it but I didn't want to change it since that would
> > >> mean that backward compatiblity and bisect-ability will be broken by this
> > >> change.
> > >>
> > >> In other words, just adding a vendor prefix won't cause an issue if patches
> > >> are merged independently since if DTS patches are merged before, the driver
> > >> will still lookup using the I2C device ID table. And if the drivers patches
> > >> are picked before, the DTS will match using the OF device ID table.
> > >>
> > >> But changing to "nokia,retu" and "nokia,tahvo" means that you will need to
> > >> pick all patches and also that the DTS and drivers changes will have to be
> > >> done in the same patch. If you are OK with that, then I can change in the
> > >> next version.
> > >
> > > tahvo is not documented nor used in any dts (in the kernel at least).
>
> True, there are no known DT users of Tahvo.
>
> > > retu is used by 1 board and happened to work, but was never documented.
> > > So I think it is okay to change unless the N800 folks object.
> >
> > I'm fine with changing it (in fact I just want to fix the I2C of modalias
> > reporting). Does this mean that backward compatibility and bisect-ability
> > should be preserved? Or it's OK to split the changes in different patches?
>
> There are 2 boards actually, N800 and N810. Retu is critical, because
> if retu-mfd/watchdog fails to probe the device will power off soon after
> boot. So for bisect-ability you should make changes in a single patch.
Also I wonder if this will work with arch/arm/mach-omap1/board-nokia770.c
that does I2C_BOARD_INFO("tahvo-mfd", 0x02). Seems they all need to be
changed with a single patch?
Regards,
Tony
Hello Aaro and Tony,
On 04/03/2017 06:58 PM, Tony Lindgren wrote:
> * Aaro Koskinen <[email protected]> [170403 15:56]:
>> Hi,
>>
>> On Mon, Apr 03, 2017 at 06:24:39PM -0400, Javier Martinez Canillas wrote:
>>> On 04/03/2017 06:20 PM, Rob Herring wrote:
>>>> On Mon, Apr 03, 2017 at 11:45:14AM -0400, Javier Martinez Canillas wrote:
>>>>> Hello Lee,
>>>>>
>>>>> On 04/03/2017 07:15 AM, Lee Jones wrote:
>>>>>
>>>>> [snip]
>>>>>
>>>>>>>
>>>>>>> +static const struct of_device_id retu_of_match[] = {
>>>>>>> + { .compatible = "nokia,retu-mfd" },
>>>>>>> + { .compatible = "nokia,tahvo-mfd" },
>>>>>>
>>>>>> Please drop the "-mfd".
>>>>>>
>>>>>
>>>>> Yes, I also didn't like it but I didn't want to change it since that would
>>>>> mean that backward compatiblity and bisect-ability will be broken by this
>>>>> change.
>>>>>
>>>>> In other words, just adding a vendor prefix won't cause an issue if patches
>>>>> are merged independently since if DTS patches are merged before, the driver
>>>>> will still lookup using the I2C device ID table. And if the drivers patches
>>>>> are picked before, the DTS will match using the OF device ID table.
>>>>>
>>>>> But changing to "nokia,retu" and "nokia,tahvo" means that you will need to
>>>>> pick all patches and also that the DTS and drivers changes will have to be
>>>>> done in the same patch. If you are OK with that, then I can change in the
>>>>> next version.
>>>>
>>>> tahvo is not documented nor used in any dts (in the kernel at least).
>>
>> True, there are no known DT users of Tahvo.
>>
>>>> retu is used by 1 board and happened to work, but was never documented.
>>>> So I think it is okay to change unless the N800 folks object.
>>>
>>> I'm fine with changing it (in fact I just want to fix the I2C of modalias
>>> reporting). Does this mean that backward compatibility and bisect-ability
>>> should be preserved? Or it's OK to split the changes in different patches?
>>
>> There are 2 boards actually, N800 and N810. Retu is critical, because
>> if retu-mfd/watchdog fails to probe the device will power off soon after
>> boot. So for bisect-ability you should make changes in a single patch.
>
Thanks for the confirmation. I'll squash the changes then in a single patch
to maintain bisect-ability.
> Also I wonder if this will work with arch/arm/mach-omap1/board-nokia770.c
> that does I2C_BOARD_INFO("tahvo-mfd", 0x02). Seems they all need to be
> changed with a single patch?
>
Yes, it has to be changed as well. So I guess that makes more sense if the
changes goes through your tree since is not only driver + DTS.
> Regards,
>
> Tony
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
Hi,
On Mon, Apr 03, 2017 at 06:24:39PM -0400, Javier Martinez Canillas wrote:
> On 04/03/2017 06:20 PM, Rob Herring wrote:
> > On Mon, Apr 03, 2017 at 11:45:14AM -0400, Javier Martinez Canillas wrote:
> >> Hello Lee,
> >>
> >> On 04/03/2017 07:15 AM, Lee Jones wrote:
> >>
> >> [snip]
> >>
> >>>>
> >>>> +static const struct of_device_id retu_of_match[] = {
> >>>> + { .compatible = "nokia,retu-mfd" },
> >>>> + { .compatible = "nokia,tahvo-mfd" },
> >>>
> >>> Please drop the "-mfd".
> >>>
> >>
> >> Yes, I also didn't like it but I didn't want to change it since that would
> >> mean that backward compatiblity and bisect-ability will be broken by this
> >> change.
> >>
> >> In other words, just adding a vendor prefix won't cause an issue if patches
> >> are merged independently since if DTS patches are merged before, the driver
> >> will still lookup using the I2C device ID table. And if the drivers patches
> >> are picked before, the DTS will match using the OF device ID table.
> >>
> >> But changing to "nokia,retu" and "nokia,tahvo" means that you will need to
> >> pick all patches and also that the DTS and drivers changes will have to be
> >> done in the same patch. If you are OK with that, then I can change in the
> >> next version.
> >
> > tahvo is not documented nor used in any dts (in the kernel at least).
True, there are no known DT users of Tahvo.
> > retu is used by 1 board and happened to work, but was never documented.
> > So I think it is okay to change unless the N800 folks object.
>
> I'm fine with changing it (in fact I just want to fix the I2C of modalias
> reporting). Does this mean that backward compatibility and bisect-ability
> should be preserved? Or it's OK to split the changes in different patches?
There are 2 boards actually, N800 and N810. Retu is critical, because
if retu-mfd/watchdog fails to probe the device will power off soon after
boot. So for bisect-ability you should make changes in a single patch.
A.
On Mon, 03 Apr 2017, Javier Martinez Canillas wrote:
> Hello Lee,
>
> On 04/03/2017 07:15 AM, Lee Jones wrote:
>
> [snip]
>
> >> +
> >> +The TP61050/TPS61052 is a high-power "white LED driver". This boost converter
> >> +is also used for other things than white LEDs, and also contains a GPIO pin.
> >
> > What functions does it offer?
> >
>
> Same comment than before, I'm not really familiar with this driver. But I'll
> look what are the MFD cell instantiated to better understand dev functions
> and expand this documentation accordingly.
Writing documentation for H/W you are unfamiliar with is a bad idea IMHO.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Hello Lee,
On 04/04/2017 04:27 AM, Lee Jones wrote:
> On Mon, 03 Apr 2017, Javier Martinez Canillas wrote:
>
>> Hello Lee,
>>
>> On 04/03/2017 07:15 AM, Lee Jones wrote:
>>
>> [snip]
>>
>>>> +
>>>> +The TP61050/TPS61052 is a high-power "white LED driver". This boost converter
>>>> +is also used for other things than white LEDs, and also contains a GPIO pin.
>>>
>>> What functions does it offer?
>>>
>>
>> Same comment than before, I'm not really familiar with this driver. But I'll
>> look what are the MFD cell instantiated to better understand dev functions
>> and expand this documentation accordingly.
>
> Writing documentation for H/W you are unfamiliar with is a bad idea IMHO.
>
I just wanted to add an OF device ID table to the driver to make sure the
driver won't have a regression when the I2C core reports an OF modalias
but was asked to also write a DT binding doc in this case...
I'm happy to drop the DT binding doc patch and someone more familiar to do
it as a follow up.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America