Re: [PATCH] ARM: dts: at91: add serial MFD sub-node for usart
On Mon, 02 Nov 2020, codrin.ciubota...@microchip.com wrote:
> On 02.11.2020 14:55, codrin.ciubota...@microchip.com wrote:
> > On 02.11.2020 14:29, Lee Jones wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> >> content is safe
> >>
> >> On Mon, 02 Nov 2020, codrin.ciubota...@microchip.com wrote:
> >>
> >>> On 02.11.2020 11:01, Lee Jones wrote:
> On Fri, 30 Oct 2020, Nicolas Ferre wrote:
>
> > On 30/10/2020 at 12:07, Codrin Ciubotariu wrote:
> >> The "atmel,at91sam9260-usart" driver is a MFD driver, so it needs
> >> sub-nodes
> >> to match the registered platform device. For this reason, we add a
> >> serial
> >> subnode to all the "atmel,at91sam9260-usart" serial compatible nods.
> >> This
> >> will also remove the boot warning:
> >> "atmel_usart_serial: Failed to locate of_node [id: -2]"
> >
> > I don't remember this warning was raised previously even if the MFD
> > driver
> > was added a while ago (Sept. 2018).
> >
> > I would say it's due to 466a62d7642f ("mfd: core: Make a best effort
> > attempt
> > to match devices with the correct of_nodes") which was added on mid
> > August
> > and corrected with 22380b65dc70 ("mfd: mfd-core: Ensure disabled
> > devices are
> > ignored without error") but maybe not covering our case.
> >
> > So, well, I don't know what's the best option to this change. Moreover,
> > I
> > would say that all other USART related properties go into the child not
> > if
> > there is a need for one.
> >
> > Lee, I suspect that we're not the only ones experiencing this ugly
> > warning
> > during the boot log: can you point us out how to deal with it for our
> > existing atmel_serial.c users?
>
> You should not be instantiating drivers through Device Tree which are
> not described there. If the correct representation of the H/W already
> exists in Device Tree i.e. no SPI and UART IP really exists, use the
> MFD core API to register them utilising the platform API instead.
>
> This should do it:
>
> diff --git a/drivers/mfd/at91-usart.c b/drivers/mfd/at91-usart.c
> index 6a8351a4588e2..939bd2332a4f6 100644
> --- a/drivers/mfd/at91-usart.c
> +++ b/drivers/mfd/at91-usart.c
> @@ -17,12 +17,10 @@
>
> static const struct mfd_cell at91_usart_spi_subdev = {
> .name = "at91_usart_spi",
> - .of_compatible = "microchip,at91sam9g45-usart-spi",
> };
>
> static const struct mfd_cell at91_usart_serial_subdev = {
> .name = "atmel_usart_serial",
> - .of_compatible = "atmel,at91rm9200-usart-serial",
> };
>
> static int at91_usart_mode_probe(struct platform_device *pdev)
> >>>
> >>> [snip]
> >>>
> >>> Hi Lee, thank you for looking through our usart driver and for sharing
> >>> your thoughts. Removing the usage of compatible string means that for
> >>> similar serial/SPI IPs we would need to create new platform drivers.
> >>
> >> Why would you need to do that?
> >
> > In the case we will have to support another similar IP, but with a
> > different set of features. Not a new platform driver from scratch, but
> > at least a new struct platform_driver for each variant.
>
> I guess we could use struct mfd_cell.platform_data to select the
> features for the serial/SPI. This platform data can be per compatible of
> our MFD driver. I will send a patch with the changes you suggested.
Yes, that is what platform data is for.
> Thank you!
NP.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH] ARM: dts: at91: add serial MFD sub-node for usart
On 02.11.2020 14:55, codrin.ciubota...@microchip.com wrote:
> On 02.11.2020 14:29, Lee Jones wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>> content is safe
>>
>> On Mon, 02 Nov 2020, codrin.ciubota...@microchip.com wrote:
>>
>>> On 02.11.2020 11:01, Lee Jones wrote:
On Fri, 30 Oct 2020, Nicolas Ferre wrote:
> On 30/10/2020 at 12:07, Codrin Ciubotariu wrote:
>> The "atmel,at91sam9260-usart" driver is a MFD driver, so it needs
>> sub-nodes
>> to match the registered platform device. For this reason, we add a serial
>> subnode to all the "atmel,at91sam9260-usart" serial compatible nods. This
>> will also remove the boot warning:
>> "atmel_usart_serial: Failed to locate of_node [id: -2]"
>
> I don't remember this warning was raised previously even if the MFD driver
> was added a while ago (Sept. 2018).
>
> I would say it's due to 466a62d7642f ("mfd: core: Make a best effort
> attempt
> to match devices with the correct of_nodes") which was added on mid August
> and corrected with 22380b65dc70 ("mfd: mfd-core: Ensure disabled devices
> are
> ignored without error") but maybe not covering our case.
>
> So, well, I don't know what's the best option to this change. Moreover, I
> would say that all other USART related properties go into the child not if
> there is a need for one.
>
> Lee, I suspect that we're not the only ones experiencing this ugly warning
> during the boot log: can you point us out how to deal with it for our
> existing atmel_serial.c users?
You should not be instantiating drivers through Device Tree which are
not described there. If the correct representation of the H/W already
exists in Device Tree i.e. no SPI and UART IP really exists, use the
MFD core API to register them utilising the platform API instead.
This should do it:
diff --git a/drivers/mfd/at91-usart.c b/drivers/mfd/at91-usart.c
index 6a8351a4588e2..939bd2332a4f6 100644
--- a/drivers/mfd/at91-usart.c
+++ b/drivers/mfd/at91-usart.c
@@ -17,12 +17,10 @@
static const struct mfd_cell at91_usart_spi_subdev = {
.name = "at91_usart_spi",
- .of_compatible = "microchip,at91sam9g45-usart-spi",
};
static const struct mfd_cell at91_usart_serial_subdev = {
.name = "atmel_usart_serial",
- .of_compatible = "atmel,at91rm9200-usart-serial",
};
static int at91_usart_mode_probe(struct platform_device *pdev)
>>>
>>> [snip]
>>>
>>> Hi Lee, thank you for looking through our usart driver and for sharing
>>> your thoughts. Removing the usage of compatible string means that for
>>> similar serial/SPI IPs we would need to create new platform drivers.
>>
>> Why would you need to do that?
>
> In the case we will have to support another similar IP, but with a
> different set of features. Not a new platform driver from scratch, but
> at least a new struct platform_driver for each variant.
I guess we could use struct mfd_cell.platform_data to select the
features for the serial/SPI. This platform data can be per compatible of
our MFD driver. I will send a patch with the changes you suggested.
Thank you!
Best regards,
Codrin
>
>>
>>> This is not ideal, but it's a solution. What I proposed is more
>>> flexible, but, as you pointed out, I am not sure it correctly describes
>>> the HW, because the decision of whether to use this IP as a serial or a
>>> SPI is a configurable one.
>>>
>>> Thanks and best regards,
>>> Codrin
>>
>> --
>> Lee Jones [李琼斯]
>> Senior Technical Lead - Developer Services
>> Linaro.org │ Open source software for Arm SoCs
>> Follow Linaro: Facebook | Twitter | Blog
>>
>
Re: [PATCH] ARM: dts: at91: add serial MFD sub-node for usart
On 02.11.2020 14:29, Lee Jones wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Mon, 02 Nov 2020, codrin.ciubota...@microchip.com wrote:
>
>> On 02.11.2020 11:01, Lee Jones wrote:
>>> On Fri, 30 Oct 2020, Nicolas Ferre wrote:
>>>
On 30/10/2020 at 12:07, Codrin Ciubotariu wrote:
> The "atmel,at91sam9260-usart" driver is a MFD driver, so it needs
> sub-nodes
> to match the registered platform device. For this reason, we add a serial
> subnode to all the "atmel,at91sam9260-usart" serial compatible nods. This
> will also remove the boot warning:
> "atmel_usart_serial: Failed to locate of_node [id: -2]"
I don't remember this warning was raised previously even if the MFD driver
was added a while ago (Sept. 2018).
I would say it's due to 466a62d7642f ("mfd: core: Make a best effort
attempt
to match devices with the correct of_nodes") which was added on mid August
and corrected with 22380b65dc70 ("mfd: mfd-core: Ensure disabled devices
are
ignored without error") but maybe not covering our case.
So, well, I don't know what's the best option to this change. Moreover, I
would say that all other USART related properties go into the child not if
there is a need for one.
Lee, I suspect that we're not the only ones experiencing this ugly warning
during the boot log: can you point us out how to deal with it for our
existing atmel_serial.c users?
>>>
>>> You should not be instantiating drivers through Device Tree which are
>>> not described there. If the correct representation of the H/W already
>>> exists in Device Tree i.e. no SPI and UART IP really exists, use the
>>> MFD core API to register them utilising the platform API instead.
>>>
>>> This should do it:
>>>
>>> diff --git a/drivers/mfd/at91-usart.c b/drivers/mfd/at91-usart.c
>>> index 6a8351a4588e2..939bd2332a4f6 100644
>>> --- a/drivers/mfd/at91-usart.c
>>> +++ b/drivers/mfd/at91-usart.c
>>> @@ -17,12 +17,10 @@
>>>
>>>static const struct mfd_cell at91_usart_spi_subdev = {
>>> .name = "at91_usart_spi",
>>> - .of_compatible = "microchip,at91sam9g45-usart-spi",
>>>};
>>>
>>>static const struct mfd_cell at91_usart_serial_subdev = {
>>> .name = "atmel_usart_serial",
>>> - .of_compatible = "atmel,at91rm9200-usart-serial",
>>>};
>>>
>>>static int at91_usart_mode_probe(struct platform_device *pdev)
>>
>> [snip]
>>
>> Hi Lee, thank you for looking through our usart driver and for sharing
>> your thoughts. Removing the usage of compatible string means that for
>> similar serial/SPI IPs we would need to create new platform drivers.
>
> Why would you need to do that?
In the case we will have to support another similar IP, but with a
different set of features. Not a new platform driver from scratch, but
at least a new struct platform_driver for each variant.
>
>> This is not ideal, but it's a solution. What I proposed is more
>> flexible, but, as you pointed out, I am not sure it correctly describes
>> the HW, because the decision of whether to use this IP as a serial or a
>> SPI is a configurable one.
>>
>> Thanks and best regards,
>> Codrin
>
> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog
>
Re: [PATCH] ARM: dts: at91: add serial MFD sub-node for usart
On 02.11.2020 11:07, Lee Jones wrote:
> On Fri, 30 Oct 2020, codrin.ciubota...@microchip.com wrote:
>
>> On 30.10.2020 15:38, Nicolas Ferre wrote:
>>> On 30/10/2020 at 12:07, Codrin Ciubotariu wrote:
The "atmel,at91sam9260-usart" driver is a MFD driver, so it needs
sub-nodes
to match the registered platform device. For this reason, we add a serial
subnode to all the "atmel,at91sam9260-usart" serial compatible nods. This
will also remove the boot warning:
"atmel_usart_serial: Failed to locate of_node [id: -2]"
>>>
>>> I don't remember this warning was raised previously even if the MFD
>>> driver was added a while ago (Sept. 2018).
>>>
>>> I would say it's due to 466a62d7642f ("mfd: core: Make a best effort
>>> attempt to match devices with the correct of_nodes") which was added on
>>> mid August and corrected with 22380b65dc70 ("mfd: mfd-core: Ensure
>>> disabled devices are ignored without error") but maybe not covering our
>>> case.
>>
>> Well, it's not covering our enabled devices.
>>
>>>
>>> So, well, I don't know what's the best option to this change. Moreover,
>>> I would say that all other USART related properties go into the child
>>> not if there is a need for one.
>>>
>>> Lee, I suspect that we're not the only ones experiencing this ugly
>>> warning during the boot log: can you point us out how to deal with it
>>> for our existing atmel_serial.c users?
>>
>> My understading is that platform devices registered by MFD should have a
>> correspondig DT node. The parrent properties are also available for the
>> other usart device (usart-spi), so I think we should keep them in the
>> parrent.
>
> Device Tree and MFD are unrelated. MFDs don't even exist - they are a
> figment of a Linux Kernel Engineer's imagination - we made them up!
>
> The DT should describe the hardware and nothing else. If we wish to
> mess with devices for our own gain i.e. organise them into different
> subsystems, we have to do that in software. That's what MFD is for.
You are right, I mixed up things. We are using the MFD here to describe
a hardware USART IP that can also function as an SPI, but not at the
same time. The decision of whether the IP works as a normal USART or an
SPI is DT configurable, at this moment. It is doing more than just
describing the HW, but I don't know how to describe it otherwise.
Best regards,
Codrin
Re: [PATCH] ARM: dts: at91: add serial MFD sub-node for usart
On Mon, 02 Nov 2020, codrin.ciubota...@microchip.com wrote:
> On 02.11.2020 11:01, Lee Jones wrote:
> > On Fri, 30 Oct 2020, Nicolas Ferre wrote:
> >
> >> On 30/10/2020 at 12:07, Codrin Ciubotariu wrote:
> >>> The "atmel,at91sam9260-usart" driver is a MFD driver, so it needs
> >>> sub-nodes
> >>> to match the registered platform device. For this reason, we add a serial
> >>> subnode to all the "atmel,at91sam9260-usart" serial compatible nods. This
> >>> will also remove the boot warning:
> >>> "atmel_usart_serial: Failed to locate of_node [id: -2]"
> >>
> >> I don't remember this warning was raised previously even if the MFD driver
> >> was added a while ago (Sept. 2018).
> >>
> >> I would say it's due to 466a62d7642f ("mfd: core: Make a best effort
> >> attempt
> >> to match devices with the correct of_nodes") which was added on mid August
> >> and corrected with 22380b65dc70 ("mfd: mfd-core: Ensure disabled devices
> >> are
> >> ignored without error") but maybe not covering our case.
> >>
> >> So, well, I don't know what's the best option to this change. Moreover, I
> >> would say that all other USART related properties go into the child not if
> >> there is a need for one.
> >>
> >> Lee, I suspect that we're not the only ones experiencing this ugly warning
> >> during the boot log: can you point us out how to deal with it for our
> >> existing atmel_serial.c users?
> >
> > You should not be instantiating drivers through Device Tree which are
> > not described there. If the correct representation of the H/W already
> > exists in Device Tree i.e. no SPI and UART IP really exists, use the
> > MFD core API to register them utilising the platform API instead.
> >
> > This should do it:
> >
> > diff --git a/drivers/mfd/at91-usart.c b/drivers/mfd/at91-usart.c
> > index 6a8351a4588e2..939bd2332a4f6 100644
> > --- a/drivers/mfd/at91-usart.c
> > +++ b/drivers/mfd/at91-usart.c
> > @@ -17,12 +17,10 @@
> >
> > static const struct mfd_cell at91_usart_spi_subdev = {
> > .name = "at91_usart_spi",
> > - .of_compatible = "microchip,at91sam9g45-usart-spi",
> > };
> >
> > static const struct mfd_cell at91_usart_serial_subdev = {
> > .name = "atmel_usart_serial",
> > - .of_compatible = "atmel,at91rm9200-usart-serial",
> > };
> >
> > static int at91_usart_mode_probe(struct platform_device *pdev)
>
> [snip]
>
> Hi Lee, thank you for looking through our usart driver and for sharing
> your thoughts. Removing the usage of compatible string means that for
> similar serial/SPI IPs we would need to create new platform drivers.
Why would you need to do that?
> This is not ideal, but it's a solution. What I proposed is more
> flexible, but, as you pointed out, I am not sure it correctly describes
> the HW, because the decision of whether to use this IP as a serial or a
> SPI is a configurable one.
>
> Thanks and best regards,
> Codrin
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH] ARM: dts: at91: add serial MFD sub-node for usart
On 02.11.2020 11:01, Lee Jones wrote:
> On Fri, 30 Oct 2020, Nicolas Ferre wrote:
>
>> On 30/10/2020 at 12:07, Codrin Ciubotariu wrote:
>>> The "atmel,at91sam9260-usart" driver is a MFD driver, so it needs sub-nodes
>>> to match the registered platform device. For this reason, we add a serial
>>> subnode to all the "atmel,at91sam9260-usart" serial compatible nods. This
>>> will also remove the boot warning:
>>> "atmel_usart_serial: Failed to locate of_node [id: -2]"
>>
>> I don't remember this warning was raised previously even if the MFD driver
>> was added a while ago (Sept. 2018).
>>
>> I would say it's due to 466a62d7642f ("mfd: core: Make a best effort attempt
>> to match devices with the correct of_nodes") which was added on mid August
>> and corrected with 22380b65dc70 ("mfd: mfd-core: Ensure disabled devices are
>> ignored without error") but maybe not covering our case.
>>
>> So, well, I don't know what's the best option to this change. Moreover, I
>> would say that all other USART related properties go into the child not if
>> there is a need for one.
>>
>> Lee, I suspect that we're not the only ones experiencing this ugly warning
>> during the boot log: can you point us out how to deal with it for our
>> existing atmel_serial.c users?
>
> You should not be instantiating drivers through Device Tree which are
> not described there. If the correct representation of the H/W already
> exists in Device Tree i.e. no SPI and UART IP really exists, use the
> MFD core API to register them utilising the platform API instead.
>
> This should do it:
>
> diff --git a/drivers/mfd/at91-usart.c b/drivers/mfd/at91-usart.c
> index 6a8351a4588e2..939bd2332a4f6 100644
> --- a/drivers/mfd/at91-usart.c
> +++ b/drivers/mfd/at91-usart.c
> @@ -17,12 +17,10 @@
>
> static const struct mfd_cell at91_usart_spi_subdev = {
> .name = "at91_usart_spi",
> - .of_compatible = "microchip,at91sam9g45-usart-spi",
> };
>
> static const struct mfd_cell at91_usart_serial_subdev = {
> .name = "atmel_usart_serial",
> - .of_compatible = "atmel,at91rm9200-usart-serial",
> };
>
> static int at91_usart_mode_probe(struct platform_device *pdev)
[snip]
Hi Lee, thank you for looking through our usart driver and for sharing
your thoughts. Removing the usage of compatible string means that for
similar serial/SPI IPs we would need to create new platform drivers.
This is not ideal, but it's a solution. What I proposed is more
flexible, but, as you pointed out, I am not sure it correctly describes
the HW, because the decision of whether to use this IP as a serial or a
SPI is a configurable one.
Thanks and best regards,
Codrin
Re: [PATCH] ARM: dts: at91: add serial MFD sub-node for usart
On Fri, 30 Oct 2020, codrin.ciubota...@microchip.com wrote:
> On 30.10.2020 15:38, Nicolas Ferre wrote:
> > On 30/10/2020 at 12:07, Codrin Ciubotariu wrote:
> >> The "atmel,at91sam9260-usart" driver is a MFD driver, so it needs
> >> sub-nodes
> >> to match the registered platform device. For this reason, we add a serial
> >> subnode to all the "atmel,at91sam9260-usart" serial compatible nods. This
> >> will also remove the boot warning:
> >> "atmel_usart_serial: Failed to locate of_node [id: -2]"
> >
> > I don't remember this warning was raised previously even if the MFD
> > driver was added a while ago (Sept. 2018).
> >
> > I would say it's due to 466a62d7642f ("mfd: core: Make a best effort
> > attempt to match devices with the correct of_nodes") which was added on
> > mid August and corrected with 22380b65dc70 ("mfd: mfd-core: Ensure
> > disabled devices are ignored without error") but maybe not covering our
> > case.
>
> Well, it's not covering our enabled devices.
>
> >
> > So, well, I don't know what's the best option to this change. Moreover,
> > I would say that all other USART related properties go into the child
> > not if there is a need for one.
> >
> > Lee, I suspect that we're not the only ones experiencing this ugly
> > warning during the boot log: can you point us out how to deal with it
> > for our existing atmel_serial.c users?
>
> My understading is that platform devices registered by MFD should have a
> correspondig DT node. The parrent properties are also available for the
> other usart device (usart-spi), so I think we should keep them in the
> parrent.
Device Tree and MFD are unrelated. MFDs don't even exist - they are a
figment of a Linux Kernel Engineer's imagination - we made them up!
The DT should describe the hardware and nothing else. If we wish to
mess with devices for our own gain i.e. organise them into different
subsystems, we have to do that in software. That's what MFD is for.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Re: [PATCH] ARM: dts: at91: add serial MFD sub-node for usart
On Fri, 30 Oct 2020, Nicolas Ferre wrote:
> On 30/10/2020 at 12:07, Codrin Ciubotariu wrote:
> > The "atmel,at91sam9260-usart" driver is a MFD driver, so it needs sub-nodes
> > to match the registered platform device. For this reason, we add a serial
> > subnode to all the "atmel,at91sam9260-usart" serial compatible nods. This
> > will also remove the boot warning:
> > "atmel_usart_serial: Failed to locate of_node [id: -2]"
>
> I don't remember this warning was raised previously even if the MFD driver
> was added a while ago (Sept. 2018).
>
> I would say it's due to 466a62d7642f ("mfd: core: Make a best effort attempt
> to match devices with the correct of_nodes") which was added on mid August
> and corrected with 22380b65dc70 ("mfd: mfd-core: Ensure disabled devices are
> ignored without error") but maybe not covering our case.
>
> So, well, I don't know what's the best option to this change. Moreover, I
> would say that all other USART related properties go into the child not if
> there is a need for one.
>
> Lee, I suspect that we're not the only ones experiencing this ugly warning
> during the boot log: can you point us out how to deal with it for our
> existing atmel_serial.c users?
You should not be instantiating drivers through Device Tree which are
not described there. If the correct representation of the H/W already
exists in Device Tree i.e. no SPI and UART IP really exists, use the
MFD core API to register them utilising the platform API instead.
This should do it:
diff --git a/drivers/mfd/at91-usart.c b/drivers/mfd/at91-usart.c
index 6a8351a4588e2..939bd2332a4f6 100644
--- a/drivers/mfd/at91-usart.c
+++ b/drivers/mfd/at91-usart.c
@@ -17,12 +17,10 @@
static const struct mfd_cell at91_usart_spi_subdev = {
.name = "at91_usart_spi",
- .of_compatible = "microchip,at91sam9g45-usart-spi",
};
static const struct mfd_cell at91_usart_serial_subdev = {
.name = "atmel_usart_serial",
- .of_compatible = "atmel,at91rm9200-usart-serial",
};
static int at91_usart_mode_probe(struct platform_device *pdev)
> > Signed-off-by: Codrin Ciubotariu
> > ---
> > arch/arm/boot/dts/at91-sam9x60ek.dts | 3 +++
> > arch/arm/boot/dts/at91rm9200.dtsi| 15
> > arch/arm/boot/dts/at91sam9261.dtsi | 12 ++
> > arch/arm/boot/dts/at91sam9261ek.dts | 3 +++
> > arch/arm/boot/dts/at91sam9263.dtsi | 12 ++
> > arch/arm/boot/dts/at91sam9g45.dtsi | 15
> > arch/arm/boot/dts/at91sam9n12.dtsi | 15
> > arch/arm/boot/dts/at91sam9rl.dtsi| 15
> > arch/arm/boot/dts/at91sam9x5.dtsi| 18 ++
> > arch/arm/boot/dts/at91sam9x5_usart3.dtsi | 3 +++
> > arch/arm/boot/dts/sam9x60.dtsi | 3 +++
> > arch/arm/boot/dts/sama5d2.dtsi | 30
> > arch/arm/boot/dts/sama5d3.dtsi | 18 ++
> > arch/arm/boot/dts/sama5d3_uart.dtsi | 6 +
> > arch/arm/boot/dts/sama5d4.dtsi | 30
> > 15 files changed, 198 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts
> > b/arch/arm/boot/dts/at91-sam9x60ek.dts
> > index eae28b82c7fd..e317531f7363 100644
> > --- a/arch/arm/boot/dts/at91-sam9x60ek.dts
> > +++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
> > @@ -280,6 +280,9 @@ AT91_XDMAC_DT_PERID(10))>,
> > atmel,use-dma-rx;
> > atmel,use-dma-tx;
> > status = "okay";
> > + serial {
> > + compatible = "atmel,at91rm9200-usart-serial";
> > + };
> > };
> > };
> > diff --git a/arch/arm/boot/dts/at91rm9200.dtsi
> > b/arch/arm/boot/dts/at91rm9200.dtsi
> > index d1181ead18e5..2abb646f6b68 100644
> > --- a/arch/arm/boot/dts/at91rm9200.dtsi
> > +++ b/arch/arm/boot/dts/at91rm9200.dtsi
> > @@ -602,6 +602,9 @@ dbgu: serial@f200 {
> > clocks = < PMC_TYPE_CORE PMC_MCK>;
> > clock-names = "usart";
> > status = "disabled";
> > + serial {
> > + compatible =
> > "atmel,at91rm9200-usart-serial";
> > + };
> > };
> > usart0: serial@fffc {
> > @@ -615,6 +618,9 @@ usart0: serial@fffc {
> > clocks = < PMC_TYPE_PERIPHERAL 6>;
> > clock-names = "usart";
> > status = "disabled";
> > + serial {
> > + compatible =
> > "atmel,at91rm9200-usart-serial";
> > + };
> > };
> > usart1: serial@fffc4000 {
> > @@ -628,6 +634,9 @@ usart1: serial@fffc4000 {
> > clocks = < PMC_TYPE_PERIPHERAL 7>;
> > clock-names = "usart";
> >
Re: [PATCH] ARM: dts: at91: add serial MFD sub-node for usart
On 30.10.2020 15:38, Nicolas Ferre wrote:
> On 30/10/2020 at 12:07, Codrin Ciubotariu wrote:
>> The "atmel,at91sam9260-usart" driver is a MFD driver, so it needs
>> sub-nodes
>> to match the registered platform device. For this reason, we add a serial
>> subnode to all the "atmel,at91sam9260-usart" serial compatible nods. This
>> will also remove the boot warning:
>> "atmel_usart_serial: Failed to locate of_node [id: -2]"
>
> I don't remember this warning was raised previously even if the MFD
> driver was added a while ago (Sept. 2018).
>
> I would say it's due to 466a62d7642f ("mfd: core: Make a best effort
> attempt to match devices with the correct of_nodes") which was added on
> mid August and corrected with 22380b65dc70 ("mfd: mfd-core: Ensure
> disabled devices are ignored without error") but maybe not covering our
> case.
Well, it's not covering our enabled devices.
>
> So, well, I don't know what's the best option to this change. Moreover,
> I would say that all other USART related properties go into the child
> not if there is a need for one.
>
> Lee, I suspect that we're not the only ones experiencing this ugly
> warning during the boot log: can you point us out how to deal with it
> for our existing atmel_serial.c users?
My understading is that platform devices registered by MFD should have a
correspondig DT node. The parrent properties are also available for the
other usart device (usart-spi), so I think we should keep them in the
parrent.
Best regards,
Codrin
Re: [PATCH] ARM: dts: at91: add serial MFD sub-node for usart
On 30/10/2020 at 12:07, Codrin Ciubotariu wrote:
The "atmel,at91sam9260-usart" driver is a MFD driver, so it needs sub-nodes
to match the registered platform device. For this reason, we add a serial
subnode to all the "atmel,at91sam9260-usart" serial compatible nods. This
will also remove the boot warning:
"atmel_usart_serial: Failed to locate of_node [id: -2]"
I don't remember this warning was raised previously even if the MFD
driver was added a while ago (Sept. 2018).
I would say it's due to 466a62d7642f ("mfd: core: Make a best effort
attempt to match devices with the correct of_nodes") which was added on
mid August and corrected with 22380b65dc70 ("mfd: mfd-core: Ensure
disabled devices are ignored without error") but maybe not covering our
case.
So, well, I don't know what's the best option to this change. Moreover,
I would say that all other USART related properties go into the child
not if there is a need for one.
Lee, I suspect that we're not the only ones experiencing this ugly
warning during the boot log: can you point us out how to deal with it
for our existing atmel_serial.c users?
Best regards,
Nicolas
Signed-off-by: Codrin Ciubotariu
---
arch/arm/boot/dts/at91-sam9x60ek.dts | 3 +++
arch/arm/boot/dts/at91rm9200.dtsi| 15
arch/arm/boot/dts/at91sam9261.dtsi | 12 ++
arch/arm/boot/dts/at91sam9261ek.dts | 3 +++
arch/arm/boot/dts/at91sam9263.dtsi | 12 ++
arch/arm/boot/dts/at91sam9g45.dtsi | 15
arch/arm/boot/dts/at91sam9n12.dtsi | 15
arch/arm/boot/dts/at91sam9rl.dtsi| 15
arch/arm/boot/dts/at91sam9x5.dtsi| 18 ++
arch/arm/boot/dts/at91sam9x5_usart3.dtsi | 3 +++
arch/arm/boot/dts/sam9x60.dtsi | 3 +++
arch/arm/boot/dts/sama5d2.dtsi | 30
arch/arm/boot/dts/sama5d3.dtsi | 18 ++
arch/arm/boot/dts/sama5d3_uart.dtsi | 6 +
arch/arm/boot/dts/sama5d4.dtsi | 30
15 files changed, 198 insertions(+)
diff --git a/arch/arm/boot/dts/at91-sam9x60ek.dts
b/arch/arm/boot/dts/at91-sam9x60ek.dts
index eae28b82c7fd..e317531f7363 100644
--- a/arch/arm/boot/dts/at91-sam9x60ek.dts
+++ b/arch/arm/boot/dts/at91-sam9x60ek.dts
@@ -280,6 +280,9 @@ AT91_XDMAC_DT_PERID(10))>,
atmel,use-dma-rx;
atmel,use-dma-tx;
status = "okay";
+ serial {
+ compatible = "atmel,at91rm9200-usart-serial";
+ };
};
};
diff --git a/arch/arm/boot/dts/at91rm9200.dtsi b/arch/arm/boot/dts/at91rm9200.dtsi
index d1181ead18e5..2abb646f6b68 100644
--- a/arch/arm/boot/dts/at91rm9200.dtsi
+++ b/arch/arm/boot/dts/at91rm9200.dtsi
@@ -602,6 +602,9 @@ dbgu: serial@f200 {
clocks = < PMC_TYPE_CORE PMC_MCK>;
clock-names = "usart";
status = "disabled";
+ serial {
+ compatible =
"atmel,at91rm9200-usart-serial";
+ };
};
usart0: serial@fffc {
@@ -615,6 +618,9 @@ usart0: serial@fffc {
clocks = < PMC_TYPE_PERIPHERAL 6>;
clock-names = "usart";
status = "disabled";
+ serial {
+ compatible =
"atmel,at91rm9200-usart-serial";
+ };
};
usart1: serial@fffc4000 {
@@ -628,6 +634,9 @@ usart1: serial@fffc4000 {
clocks = < PMC_TYPE_PERIPHERAL 7>;
clock-names = "usart";
status = "disabled";
+ serial {
+ compatible =
"atmel,at91rm9200-usart-serial";
+ };
};
usart2: serial@fffc8000 {
@@ -641,6 +650,9 @@ usart2: serial@fffc8000 {
clocks = < PMC_TYPE_PERIPHERAL 8>;
clock-names = "usart";
status = "disabled";
+ serial {
+ compatible =
"atmel,at91rm9200-usart-serial";
+ };
};
usart3: serial@fffcc000 {
@@ -654,6 +666,9 @@ usart3: serial@fffcc000 {
clocks = < PMC_TYPE_PERIPHERAL 9>;
clock-names = "usart";
status = "disabled";
+ serial {
+ compatible =
"atmel,at91rm9200-usart-serial";
+ };
