| Message ID | 20180726065331.6186-5-o.rempel@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add mailbox support for i.MX7D | expand |
> -----Original Message----- > From: Oleksij Rempel [mailto:o.rempel@pengutronix.de] > Sent: Thursday, July 26, 2018 2:53 PM > To: Shawn Guo <shawnguo@kernel.org>; Fabio Estevam > <fabio.estevam@nxp.com>; Rob Herring <robh+dt@kernel.org>; Mark > Rutland <mark.rutland@arm.com>; A.s. Dong <aisheng.dong@nxp.com>; > Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>; Jassi Brar > <jassisinghbrar@gmail.com> > Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de; > linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; dl-linux- > imx <linux-imx@nxp.com> > Subject: [PATCH v7 4/6] dt-bindings: mailbox: imx-mu: add i.MX6SX and > i.MX7S SoCs. > > This are currently tested SoCs with imx-mailbox driver. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com> Regards Dong Aisheng
On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel <o.rempel@pengutronix.de> wrote: > This are currently tested SoCs with imx-mailbox driver. > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > index 113d6ab931ef..5616d2afca45 100644 > --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > @@ -18,7 +18,7 @@ Messaging Unit Device Node: > Required properties: > ------------------- > - compatible : should be "fsl,<chip>-mu", the supported chips include > - imx8qxp, imx8qm. > + imx6sx, imx7s, imx8qxp, imx8qm. > This is not scalable. Do we add every new SoC that contains the same controller? I think the controller name, if it has one or create a new one like 'imx-mu', should be used here. So, compatible : should be "fsl,imx-mu" BTW, the driver doesn't even probe anything other than 'fsl,imx6sx-mu', unlike what the binding says.
Hi Jassi, Am Donnerstag, den 26.07.2018, 15:25 +0530 schrieb Jassi Brar: > On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel > > <o.rempel@pengutronix.de> wrote: > > This are currently tested SoCs with imx-mailbox driver. > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > index 113d6ab931ef..5616d2afca45 100644 > > --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > @@ -18,7 +18,7 @@ Messaging Unit Device Node: > > Required properties: > > ------------------- > > - compatible : should be "fsl,<chip>-mu", the supported chips include > > - imx8qxp, imx8qm. > > + imx6sx, imx7s, imx8qxp, imx8qm. > > > > This is not scalable. Do we add every new SoC that contains the same controller? Yes, we do. This is a policy direction from the DT maintainers. If we ever going to want to validate DTs against the binding, all compatibles used in the DTs must be specified in the binding. As we can't really tell if the controller is exactly the same or even has some SoC integration bugs, we generally add a new compatible for each SoC to key off any workarounds necessary in the driver without the need to change the DTs, breaking compatibility. Regards, Lucas
On Thu, Jul 26, 2018 at 4:11 PM, Lucas Stach <l.stach@pengutronix.de> wrote: > Hi Jassi, > > Am Donnerstag, den 26.07.2018, 15:25 +0530 schrieb Jassi Brar: >> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel >> > <o.rempel@pengutronix.de> wrote: >> > This are currently tested SoCs with imx-mailbox driver. >> > >> > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >> > --- >> > Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > index 113d6ab931ef..5616d2afca45 100644 >> > --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > @@ -18,7 +18,7 @@ Messaging Unit Device Node: >> > Required properties: >> > ------------------- >> > - compatible : should be "fsl,<chip>-mu", the supported chips include >> > - imx8qxp, imx8qm. >> > + imx6sx, imx7s, imx8qxp, imx8qm. >> > >> >> This is not scalable. Do we add every new SoC that contains the same controller? > > Yes, we do. This is a policy direction from the DT maintainers. > I would love to read the post/documentation. Consider the same h/w - controller and platforms, but only the the MU chapter said the controller name is, say, 'MU121'. I am sure now you will see it correct to call it "fsl,mu121" compatible. What changed? just the name, right? > If we > ever going to want to validate DTs against the binding, all compatibles > used in the DTs must be specified in the binding. > > As we can't really tell if the controller is exactly the same or even > has some SoC integration bugs, we generally add a new compatible for > each SoC to key off any workarounds necessary in the driver without the > need to change the DTs, breaking compatibility. > I think if the h/w resources and behaviour remain the same and the documentation does not call it by a different name -- it is safe to assume its the same IP. Especially when the driver is absolutely indifferent to the 5 SoC names. If/when we find the controller changes, we could revisit the binding and add another compatible option and modify the driver to catch that and adapt.
Am Donnerstag, den 26.07.2018, 16:45 +0530 schrieb Jassi Brar: > On Thu, Jul 26, 2018 at 4:11 PM, Lucas Stach <l.stach@pengutronix.de> > wrote: > > Hi Jassi, > > > > Am Donnerstag, den 26.07.2018, 15:25 +0530 schrieb Jassi Brar: > > > On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel > > > > <o.rempel@pengutronix.de> wrote: > > > > This are currently tested SoCs with imx-mailbox driver. > > > > > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > > > > > > --- > > > > Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > > > b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > > > index 113d6ab931ef..5616d2afca45 100644 > > > > --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > > > +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > > > @@ -18,7 +18,7 @@ Messaging Unit Device Node: > > > > Required properties: > > > > ------------------- > > > > - compatible : should be "fsl,<chip>-mu", the supported chips > > > > include > > > > - imx8qxp, imx8qm. > > > > + imx6sx, imx7s, imx8qxp, imx8qm. > > > > > > > > > > This is not scalable. Do we add every new SoC that contains the > > > same controller? > > > > Yes, we do. This is a policy direction from the DT maintainers. > > > > I would love to read the post/documentation. > > Consider the same h/w - controller and platforms, but only the the MU > chapter said the controller name is, say, 'MU121'. I am sure now you > will see it correct to call it "fsl,mu121" compatible. > What changed? just the name, right? > > > > If we > > ever going to want to validate DTs against the binding, all > > compatibles > > used in the DTs must be specified in the binding. > > > > As we can't really tell if the controller is exactly the same or > > even > > has some SoC integration bugs, we generally add a new compatible > > for > > each SoC to key off any workarounds necessary in the driver without > > the > > need to change the DTs, breaking compatibility. > > > > I think if the h/w resources and behaviour remain the same and the > documentation does not call it by a different name -- it is safe to > assume its the same IP. Especially when the driver is absolutely > indifferent to the 5 SoC names. Even if it is the same IP core, the SoC integration might have bugs that need different behavior from the driver. We've already had that case with the i.MX6 SPI controller. > If/when we find the controller changes, we could revisit the binding > and add another compatible option and modify the driver to catch that > and adapt. That's way too late. If we want to keep DTs stable the DT must include forward looking compatibles, to allow the driver to key any required behavior changes from already present compatibles. Otherwise you require the DT to be updated in step-lock with the kernel or need much more awkward detection in the drivers. IP integration bugs might be found way after the initial DT has been written. Regards, Lucas
On 07/26/2018 02:15 PM, Jassi Brar wrote: > On Thu, Jul 26, 2018 at 4:11 PM, Lucas Stach <l.stach@pengutronix.de> wrote: >> Hi Jassi, >> >> Am Donnerstag, den 26.07.2018, 15:25 +0530 schrieb Jassi Brar: >>> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel >>>> <o.rempel@pengutronix.de> wrote: >>>> This are currently tested SoCs with imx-mailbox driver. >>>> >>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>> --- >>>> Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>> index 113d6ab931ef..5616d2afca45 100644 >>>> --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>> +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>> @@ -18,7 +18,7 @@ Messaging Unit Device Node: >>>> Required properties: >>>> ------------------- >>>> - compatible : should be "fsl,<chip>-mu", the supported chips include >>>> - imx8qxp, imx8qm. >>>> + imx6sx, imx7s, imx8qxp, imx8qm. >>>> >>> >>> This is not scalable. Do we add every new SoC that contains the same controller? >> >> Yes, we do. This is a policy direction from the DT maintainers. >> > I would love to read the post/documentation. Some notes are found in Documentation/devicetree/bindings/ABI.txt To guarantee "a stable binding" of a compatible property, to avoid unintentionally added incompatibilities and to fix a list of supported compatibles in the device driver code there should be a SoC specific compatible value in the list of 'compatible' property elements. > Consider the same h/w - controller and platforms, but only the the MU > chapter said the controller name is, say, 'MU121'. I am sure now you > will see it correct to call it "fsl,mu121" compatible. > What changed? just the name, right? > > >> If we >> ever going to want to validate DTs against the binding, all compatibles >> used in the DTs must be specified in the binding. >> >> As we can't really tell if the controller is exactly the same or even >> has some SoC integration bugs, we generally add a new compatible for >> each SoC to key off any workarounds necessary in the driver without the >> need to change the DTs, breaking compatibility. >> > I think if the h/w resources and behaviour remain the same and the > documentation does not call it by a different name -- it is safe to > assume its the same IP. Especially when the driver is absolutely > indifferent to the 5 SoC names. > > If/when we find the controller changes, we could revisit the binding > and add another compatible option and modify the driver to catch that > and adapt. > Unfortunately it does not work well this way due to limited possibilities to distinugush different device IPs on different SoCs, if identical compatibles are given in both cases, and often it is not obvious that two IPs are different. -- Best wishes, Vladimir
On Thu, Jul 26, 2018 at 5:05 PM, Lucas Stach <l.stach@pengutronix.de> wrote: > Am Donnerstag, den 26.07.2018, 16:45 +0530 schrieb Jassi Brar: >> On Thu, Jul 26, 2018 at 4:11 PM, Lucas Stach <l.stach@pengutronix.de> >> wrote: >> > Hi Jassi, >> > >> > Am Donnerstag, den 26.07.2018, 15:25 +0530 schrieb Jassi Brar: >> > > On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel >> > > > <o.rempel@pengutronix.de> wrote: >> > > > This are currently tested SoCs with imx-mailbox driver. >> > > > >> > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >> > > > >> > > > --- >> > > > Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- >> > > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > > >> > > > diff --git >> > > > a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > > > b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > > > index 113d6ab931ef..5616d2afca45 100644 >> > > > --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > > > +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > > > @@ -18,7 +18,7 @@ Messaging Unit Device Node: >> > > > Required properties: >> > > > ------------------- >> > > > - compatible : should be "fsl,<chip>-mu", the supported chips >> > > > include >> > > > - imx8qxp, imx8qm. >> > > > + imx6sx, imx7s, imx8qxp, imx8qm. >> > > > >> > > >> > > This is not scalable. Do we add every new SoC that contains the >> > > same controller? >> > >> > Yes, we do. This is a policy direction from the DT maintainers. >> > >> >> I would love to read the post/documentation. >> >> Consider the same h/w - controller and platforms, but only the the MU >> chapter said the controller name is, say, 'MU121'. I am sure now you >> will see it correct to call it "fsl,mu121" compatible. >> What changed? just the name, right? >> >> >> > If we >> > ever going to want to validate DTs against the binding, all >> > compatibles >> > used in the DTs must be specified in the binding. >> > >> > As we can't really tell if the controller is exactly the same or >> > even >> > has some SoC integration bugs, we generally add a new compatible >> > for >> > each SoC to key off any workarounds necessary in the driver without >> > the >> > need to change the DTs, breaking compatibility. >> > >> >> I think if the h/w resources and behaviour remain the same and the >> documentation does not call it by a different name -- it is safe to >> assume its the same IP. Especially when the driver is absolutely >> indifferent to the 5 SoC names. > > Even if it is the same IP core, the SoC integration might have bugs > that need different behavior from the driver. We've already had that > case with the i.MX6 SPI controller. > For h/w quirks/bugs, a new "has-that-bug" property makes better sense. Or, if you insist, a new compatible based on the first soc that has the buggy block. >> If/when we find the controller changes, we could revisit the binding >> and add another compatible option and modify the driver to catch that >> and adapt. > > That's way too late. If we want to keep DTs stable > How do you keep the DT stable by explicitly defining every new SoC to the compatible list in DT, _add_ to the driver.... only to have the driver absolutely not care which SoC is it? Which is the situation right now with this patchset. thnx.
On 07/26/2018 02:46 PM, Jassi Brar wrote: > On Thu, Jul 26, 2018 at 5:05 PM, Lucas Stach <l.stach@pengutronix.de> wrote: >> Am Donnerstag, den 26.07.2018, 16:45 +0530 schrieb Jassi Brar: >>> On Thu, Jul 26, 2018 at 4:11 PM, Lucas Stach <l.stach@pengutronix.de> >>> wrote: >>>> Hi Jassi, >>>> >>>> Am Donnerstag, den 26.07.2018, 15:25 +0530 schrieb Jassi Brar: >>>>> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel >>>>>> <o.rempel@pengutronix.de> wrote: >>>>>> This are currently tested SoCs with imx-mailbox driver. >>>>>> >>>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>>>> >>>>>> --- >>>>>> Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git >>>>>> a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>> b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>> index 113d6ab931ef..5616d2afca45 100644 >>>>>> --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>> +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>> @@ -18,7 +18,7 @@ Messaging Unit Device Node: >>>>>> Required properties: >>>>>> ------------------- >>>>>> - compatible : should be "fsl,<chip>-mu", the supported chips >>>>>> include >>>>>> - imx8qxp, imx8qm. >>>>>> + imx6sx, imx7s, imx8qxp, imx8qm. >>>>>> >>>>> >>>>> This is not scalable. Do we add every new SoC that contains the >>>>> same controller? >>>> >>>> Yes, we do. This is a policy direction from the DT maintainers. >>>> >>> >>> I would love to read the post/documentation. >>> >>> Consider the same h/w - controller and platforms, but only the the MU >>> chapter said the controller name is, say, 'MU121'. I am sure now you >>> will see it correct to call it "fsl,mu121" compatible. >>> What changed? just the name, right? >>> >>> >>>> If we >>>> ever going to want to validate DTs against the binding, all >>>> compatibles >>>> used in the DTs must be specified in the binding. >>>> >>>> As we can't really tell if the controller is exactly the same or >>>> even >>>> has some SoC integration bugs, we generally add a new compatible >>>> for >>>> each SoC to key off any workarounds necessary in the driver without >>>> the >>>> need to change the DTs, breaking compatibility. >>>> >>> >>> I think if the h/w resources and behaviour remain the same and the >>> documentation does not call it by a different name -- it is safe to >>> assume its the same IP. Especially when the driver is absolutely >>> indifferent to the 5 SoC names. >> >> Even if it is the same IP core, the SoC integration might have bugs >> that need different behavior from the driver. We've already had that >> case with the i.MX6 SPI controller. >> > For h/w quirks/bugs, a new "has-that-bug" property makes better sense. > Or, if you insist, a new compatible based on the first soc that has > the buggy block. > How do you propose to handle this property in the driver, if once flashed DTB does not contain it? >>> If/when we find the controller changes, we could revisit the binding >>> and add another compatible option and modify the driver to catch that >>> and adapt. >> >> That's way too late. If we want to keep DTs stable >> > How do you keep the DT stable by explicitly defining every new SoC to > the compatible list in DT, _add_ to the driver.... only to have the > driver absolutely not care which SoC is it? > Which is the situation right now with this patchset. > For sake of simplicity you can assume that a driver can be changed in future, and SoC/board device tree bindings can not be changed ever. -- Best wishes, Vladimir
On Thu, Jul 26, 2018 at 5:14 PM, Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> wrote: > On 07/26/2018 02:15 PM, Jassi Brar wrote: >> On Thu, Jul 26, 2018 at 4:11 PM, Lucas Stach <l.stach@pengutronix.de> wrote: >>> Hi Jassi, >>> >>> Am Donnerstag, den 26.07.2018, 15:25 +0530 schrieb Jassi Brar: >>>> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel >>>>> <o.rempel@pengutronix.de> wrote: >>>>> This are currently tested SoCs with imx-mailbox driver. >>>>> >>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>>> --- >>>>> Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>> index 113d6ab931ef..5616d2afca45 100644 >>>>> --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>> +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>> @@ -18,7 +18,7 @@ Messaging Unit Device Node: >>>>> Required properties: >>>>> ------------------- >>>>> - compatible : should be "fsl,<chip>-mu", the supported chips include >>>>> - imx8qxp, imx8qm. >>>>> + imx6sx, imx7s, imx8qxp, imx8qm. >>>>> >>>> >>>> This is not scalable. Do we add every new SoC that contains the same controller? >>> >>> Yes, we do. This is a policy direction from the DT maintainers. >>> >> I would love to read the post/documentation. > > Some notes are found in Documentation/devicetree/bindings/ABI.txt > Thanks. I have been through the ABI.txt. > To guarantee "a stable binding" of a compatible property, to avoid > unintentionally added incompatibilities and to fix a list of supported > compatibles in the device driver code there should be a SoC specific > compatible value in the list of 'compatible' property elements. > >> Consider the same h/w - controller and platforms, but only the the MU >> chapter said the controller name is, say, 'MU121'. I am sure now you >> will see it correct to call it "fsl,mu121" compatible. >> What changed? just the name, right? >> >> >>> If we >>> ever going to want to validate DTs against the binding, all compatibles >>> used in the DTs must be specified in the binding. >>> >>> As we can't really tell if the controller is exactly the same or even >>> has some SoC integration bugs, we generally add a new compatible for >>> each SoC to key off any workarounds necessary in the driver without the >>> need to change the DTs, breaking compatibility. >>> >> I think if the h/w resources and behaviour remain the same and the >> documentation does not call it by a different name -- it is safe to >> assume its the same IP. Especially when the driver is absolutely >> indifferent to the 5 SoC names. >> >> If/when we find the controller changes, we could revisit the binding >> and add another compatible option and modify the driver to catch that >> and adapt. >> > > Unfortunately it does not work well this way due to limited possibilities > to distinugush different device IPs on different SoCs, if identical > compatibles are given in both cases, and often it is not obvious that > two IPs are different. > Please note the submitted driver absolutely don't care which of the five SoCs it is. In other words, all these SoCs have the same controller. So its about have just one compatible right now, and add more if some new SoC comes with a variation of the controller.
Am Donnerstag, den 26.07.2018, 17:16 +0530 schrieb Jassi Brar: > > On Thu, Jul 26, 2018 at 5:05 PM, Lucas Stach <l.stach@pengutronix.de> wrote: > > Am Donnerstag, den 26.07.2018, 16:45 +0530 schrieb Jassi Brar: > > > On Thu, Jul 26, 2018 at 4:11 PM, Lucas Stach <l.stach@pengutronix.de> > > > wrote: > > > > Hi Jassi, > > > > > > > > Am Donnerstag, den 26.07.2018, 15:25 +0530 schrieb Jassi Brar: > > > > > On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel > > > > > > > > > > > > <o.rempel@pengutronix.de> wrote: > > > > > > This are currently tested SoCs with imx-mailbox driver. > > > > > > > > > > > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > > > > > > > > > > > --- > > > > > > Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git > > > > > > a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > > > > > b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > > > > > index 113d6ab931ef..5616d2afca45 100644 > > > > > > --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > > > > > +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > > > > > @@ -18,7 +18,7 @@ Messaging Unit Device Node: > > > > > > Required properties: > > > > > > ------------------- > > > > > > - compatible : should be "fsl,<chip>-mu", the supported chips > > > > > > include > > > > > > - imx8qxp, imx8qm. > > > > > > + imx6sx, imx7s, imx8qxp, imx8qm. > > > > > > > > > > > > > > > > This is not scalable. Do we add every new SoC that contains the > > > > > same controller? > > > > > > > > Yes, we do. This is a policy direction from the DT maintainers. > > > > > > > > > > I would love to read the post/documentation. > > > > > > Consider the same h/w - controller and platforms, but only the the MU > > > chapter said the controller name is, say, 'MU121'. I am sure now you > > > will see it correct to call it "fsl,mu121" compatible. > > > What changed? just the name, right? > > > > > > > > > > If we > > > > ever going to want to validate DTs against the binding, all > > > > compatibles > > > > used in the DTs must be specified in the binding. > > > > > > > > As we can't really tell if the controller is exactly the same or > > > > even > > > > has some SoC integration bugs, we generally add a new compatible > > > > for > > > > each SoC to key off any workarounds necessary in the driver without > > > > the > > > > need to change the DTs, breaking compatibility. > > > > > > > > > > I think if the h/w resources and behaviour remain the same and the > > > documentation does not call it by a different name -- it is safe to > > > assume its the same IP. Especially when the driver is absolutely > > > indifferent to the 5 SoC names. > > > > Even if it is the same IP core, the SoC integration might have bugs > > that need different behavior from the driver. We've already had that > > case with the i.MX6 SPI controller. > > > > For h/w quirks/bugs, a new "has-that-bug" property makes better sense. > Or, if you insist, a new compatible based on the first soc that has > the buggy block. > Hardware bugs might not be known at the the time the DT is deployed. The DT is not part of the kernel, but might be part of a fixed device firmware. > > > If/when we find the controller changes, we could revisit the binding > > > and add another compatible option and modify the driver to catch that > > > and adapt. > > > > That's way too late. If we want to keep DTs stable > > > > How do you keep the DT stable by explicitly defining every new SoC to > the compatible list in DT, _add_ to the driver.... only to have the > driver absolutely not care which SoC is it? > Which is the situation right now with this patchset. It means we can later, in a newer kernel change the driver to use the compatible to change behavior without the need to update the DT at that point. Regards, Lucas
On 07/26/2018 02:52 PM, Jassi Brar wrote: > On Thu, Jul 26, 2018 at 5:14 PM, Vladimir Zapolskiy > <vladimir_zapolskiy@mentor.com> wrote: >> On 07/26/2018 02:15 PM, Jassi Brar wrote: >>> On Thu, Jul 26, 2018 at 4:11 PM, Lucas Stach <l.stach@pengutronix.de> wrote: >>>> Hi Jassi, >>>> >>>> Am Donnerstag, den 26.07.2018, 15:25 +0530 schrieb Jassi Brar: >>>>> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel >>>>>> <o.rempel@pengutronix.de> wrote: >>>>>> This are currently tested SoCs with imx-mailbox driver. >>>>>> >>>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>>>> --- >>>>>> Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>> index 113d6ab931ef..5616d2afca45 100644 >>>>>> --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>> +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>> @@ -18,7 +18,7 @@ Messaging Unit Device Node: >>>>>> Required properties: >>>>>> ------------------- >>>>>> - compatible : should be "fsl,<chip>-mu", the supported chips include >>>>>> - imx8qxp, imx8qm. >>>>>> + imx6sx, imx7s, imx8qxp, imx8qm. >>>>>> >>>>> >>>>> This is not scalable. Do we add every new SoC that contains the same controller? >>>> >>>> Yes, we do. This is a policy direction from the DT maintainers. >>>> >>> I would love to read the post/documentation. >> >> Some notes are found in Documentation/devicetree/bindings/ABI.txt >> > Thanks. I have been through the ABI.txt. > >> To guarantee "a stable binding" of a compatible property, to avoid >> unintentionally added incompatibilities and to fix a list of supported >> compatibles in the device driver code there should be a SoC specific >> compatible value in the list of 'compatible' property elements. >> >>> Consider the same h/w - controller and platforms, but only the the MU >>> chapter said the controller name is, say, 'MU121'. I am sure now you >>> will see it correct to call it "fsl,mu121" compatible. >>> What changed? just the name, right? >>> >>> >>>> If we >>>> ever going to want to validate DTs against the binding, all compatibles >>>> used in the DTs must be specified in the binding. >>>> >>>> As we can't really tell if the controller is exactly the same or even >>>> has some SoC integration bugs, we generally add a new compatible for >>>> each SoC to key off any workarounds necessary in the driver without the >>>> need to change the DTs, breaking compatibility. >>>> >>> I think if the h/w resources and behaviour remain the same and the >>> documentation does not call it by a different name -- it is safe to >>> assume its the same IP. Especially when the driver is absolutely >>> indifferent to the 5 SoC names. >>> >>> If/when we find the controller changes, we could revisit the binding >>> and add another compatible option and modify the driver to catch that >>> and adapt. >>> >> >> Unfortunately it does not work well this way due to limited possibilities >> to distinugush different device IPs on different SoCs, if identical >> compatibles are given in both cases, and often it is not obvious that >> two IPs are different. >> > Please note the submitted driver absolutely don't care which of the > five SoCs it is. True. > In other words, all these SoCs have the same controller. False :) > So its about have just one compatible right now, and add more if some > new SoC comes with a variation of the controller. > True. The driver will be changed in this case, unfortunately the bindings are not so volatile. -- Best wishes, Vladimir
On Thu, Jul 26, 2018 at 5:21 PM, Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> wrote: > On 07/26/2018 02:46 PM, Jassi Brar wrote: >> On Thu, Jul 26, 2018 at 5:05 PM, Lucas Stach <l.stach@pengutronix.de> wrote: >>> Am Donnerstag, den 26.07.2018, 16:45 +0530 schrieb Jassi Brar: >>>> On Thu, Jul 26, 2018 at 4:11 PM, Lucas Stach <l.stach@pengutronix.de> >>>> wrote: >>>>> Hi Jassi, >>>>> >>>>> Am Donnerstag, den 26.07.2018, 15:25 +0530 schrieb Jassi Brar: >>>>>> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel >>>>>>> <o.rempel@pengutronix.de> wrote: >>>>>>> This are currently tested SoCs with imx-mailbox driver. >>>>>>> >>>>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>>>>> >>>>>>> --- >>>>>>> Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- >>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git >>>>>>> a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>>> b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>>> index 113d6ab931ef..5616d2afca45 100644 >>>>>>> --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>>> +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>>> @@ -18,7 +18,7 @@ Messaging Unit Device Node: >>>>>>> Required properties: >>>>>>> ------------------- >>>>>>> - compatible : should be "fsl,<chip>-mu", the supported chips >>>>>>> include >>>>>>> - imx8qxp, imx8qm. >>>>>>> + imx6sx, imx7s, imx8qxp, imx8qm. >>>>>>> >>>>>> >>>>>> This is not scalable. Do we add every new SoC that contains the >>>>>> same controller? >>>>> >>>>> Yes, we do. This is a policy direction from the DT maintainers. >>>>> >>>> >>>> I would love to read the post/documentation. >>>> >>>> Consider the same h/w - controller and platforms, but only the the MU >>>> chapter said the controller name is, say, 'MU121'. I am sure now you >>>> will see it correct to call it "fsl,mu121" compatible. >>>> What changed? just the name, right? >>>> >>>> >>>>> If we >>>>> ever going to want to validate DTs against the binding, all >>>>> compatibles >>>>> used in the DTs must be specified in the binding. >>>>> >>>>> As we can't really tell if the controller is exactly the same or >>>>> even >>>>> has some SoC integration bugs, we generally add a new compatible >>>>> for >>>>> each SoC to key off any workarounds necessary in the driver without >>>>> the >>>>> need to change the DTs, breaking compatibility. >>>>> >>>> >>>> I think if the h/w resources and behaviour remain the same and the >>>> documentation does not call it by a different name -- it is safe to >>>> assume its the same IP. Especially when the driver is absolutely >>>> indifferent to the 5 SoC names. >>> >>> Even if it is the same IP core, the SoC integration might have bugs >>> that need different behavior from the driver. We've already had that >>> case with the i.MX6 SPI controller. >>> >> For h/w quirks/bugs, a new "has-that-bug" property makes better sense. >> Or, if you insist, a new compatible based on the first soc that has >> the buggy block. >> > > How do you propose to handle this property in the driver, if once flashed > DTB does not contain it? > I see, your point is - what if a bug appears only in a particular SoC ? Well, if the controller is same the bug should appear to all SoCs. Do you think these SoCs have different versions of MU ?
On 07/26/2018 03:00 PM, Jassi Brar wrote: > On Thu, Jul 26, 2018 at 5:21 PM, Vladimir Zapolskiy > <vladimir_zapolskiy@mentor.com> wrote: >> On 07/26/2018 02:46 PM, Jassi Brar wrote: >>> On Thu, Jul 26, 2018 at 5:05 PM, Lucas Stach <l.stach@pengutronix.de> wrote: >>>> Am Donnerstag, den 26.07.2018, 16:45 +0530 schrieb Jassi Brar: >>>>> On Thu, Jul 26, 2018 at 4:11 PM, Lucas Stach <l.stach@pengutronix.de> >>>>> wrote: >>>>>> Hi Jassi, >>>>>> >>>>>> Am Donnerstag, den 26.07.2018, 15:25 +0530 schrieb Jassi Brar: >>>>>>> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel >>>>>>>> <o.rempel@pengutronix.de> wrote: >>>>>>>> This are currently tested SoCs with imx-mailbox driver. >>>>>>>> >>>>>>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >>>>>>>> >>>>>>>> --- >>>>>>>> Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- >>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git >>>>>>>> a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>>>> b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>>>> index 113d6ab931ef..5616d2afca45 100644 >>>>>>>> --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>>>> +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >>>>>>>> @@ -18,7 +18,7 @@ Messaging Unit Device Node: >>>>>>>> Required properties: >>>>>>>> ------------------- >>>>>>>> - compatible : should be "fsl,<chip>-mu", the supported chips >>>>>>>> include >>>>>>>> - imx8qxp, imx8qm. >>>>>>>> + imx6sx, imx7s, imx8qxp, imx8qm. >>>>>>>> >>>>>>> >>>>>>> This is not scalable. Do we add every new SoC that contains the >>>>>>> same controller? >>>>>> >>>>>> Yes, we do. This is a policy direction from the DT maintainers. >>>>>> >>>>> >>>>> I would love to read the post/documentation. >>>>> >>>>> Consider the same h/w - controller and platforms, but only the the MU >>>>> chapter said the controller name is, say, 'MU121'. I am sure now you >>>>> will see it correct to call it "fsl,mu121" compatible. >>>>> What changed? just the name, right? >>>>> >>>>> >>>>>> If we >>>>>> ever going to want to validate DTs against the binding, all >>>>>> compatibles >>>>>> used in the DTs must be specified in the binding. >>>>>> >>>>>> As we can't really tell if the controller is exactly the same or >>>>>> even >>>>>> has some SoC integration bugs, we generally add a new compatible >>>>>> for >>>>>> each SoC to key off any workarounds necessary in the driver without >>>>>> the >>>>>> need to change the DTs, breaking compatibility. >>>>>> >>>>> >>>>> I think if the h/w resources and behaviour remain the same and the >>>>> documentation does not call it by a different name -- it is safe to >>>>> assume its the same IP. Especially when the driver is absolutely >>>>> indifferent to the 5 SoC names. >>>> >>>> Even if it is the same IP core, the SoC integration might have bugs >>>> that need different behavior from the driver. We've already had that >>>> case with the i.MX6 SPI controller. >>>> >>> For h/w quirks/bugs, a new "has-that-bug" property makes better sense. >>> Or, if you insist, a new compatible based on the first soc that has >>> the buggy block. >>> >> >> How do you propose to handle this property in the driver, if once flashed >> DTB does not contain it? >> > I see, your point is - what if a bug appears only in a particular SoC ? > Well, if the controller is same the bug should appear to all SoCs. > Do you think these SoCs have different versions of MU ? > I'm not a hardware developer, but I'm quite sure that strictly speaking IPs are different on different SoCs, the diversity of differences can be negligible, or it can be discovered in future and considered as significant, i.e. two IPs may be found as incompatible ones. Note that the DTBs flashed on boards are already prepared for the worst scenario. But to some extend, and in today's knowledge, the IPs are compatible, let's say they are compatible to "fsl,imx6sx-mu" version, the one which is found in the published driver. -- Best wishes, Vladimir
On Thu, Jul 26, 2018 at 5:25 PM, Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> wrote: > On 07/26/2018 02:52 PM, Jassi Brar wrote: >> On Thu, Jul 26, 2018 at 5:14 PM, Vladimir Zapolskiy >> <vladimir_zapolskiy@mentor.com> wrote: >>> >> Please note the submitted driver absolutely don't care which of the >> five SoCs it is. > > True. > >> In other words, all these SoCs have the same controller. > > False :) > OK, so the controllers are not identical, but same enough to have a common driver? Is the driver not tested enough or are you planning to add more features? >> So its about have just one compatible right now, and add more if some >> new SoC comes with a variation of the controller. >> > > True. The driver will be changed in this case, unfortunately the bindings > are not so volatile. > The volatility of the bindings will be same when you add a new SoC compatible ;) And you must add to bindings+driver, or the same h/w and same driver won't work for your new platform.
On 07/26/2018 03:10 PM, Jassi Brar wrote: > On Thu, Jul 26, 2018 at 5:25 PM, Vladimir Zapolskiy > <vladimir_zapolskiy@mentor.com> wrote: >> On 07/26/2018 02:52 PM, Jassi Brar wrote: >>> On Thu, Jul 26, 2018 at 5:14 PM, Vladimir Zapolskiy >>> <vladimir_zapolskiy@mentor.com> wrote: > >>>> >>> Please note the submitted driver absolutely don't care which of the >>> five SoCs it is. >> >> True. >> > >>> In other words, all these SoCs have the same controller. >> >> False :) >> To be more precice, the statement itself may be true or false, but the implication ("In other words, ...") is definitely false. > OK, so the controllers are not identical, but same enough to have a > common driver? Right, the controllers are compatible, but likely they are non-identical. > Is the driver not tested enough or are you planning to add more features? > Test results can not serve as a formal eternal proof, but they are good as a hint. Also I'm not aware of any pending features to be added to the driver. >>> So its about have just one compatible right now, and add more if some >>> new SoC comes with a variation of the controller. >>> >> >> True. The driver will be changed in this case, unfortunately the bindings >> are not so volatile. >> > The volatility of the bindings will be same when you add a new SoC > compatible ;) And you must add to bindings+driver, or the same h/w > and same driver won't work for your new platform. > The (non-)volatilitily of the bindings is important only in retrospective view. Adding a new compatible to the documentation and unrelated board DTBs is irrelevant, and it is quite predictable that it will happen, when a new SoC with the same "compatible" IP is released. -- Best wishes, Vladimir
On Thu, Jul 26, 2018 at 03:25:15PM +0530, Jassi Brar wrote: > On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel > <o.rempel@pengutronix.de> wrote: > > This are currently tested SoCs with imx-mailbox driver. > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > index 113d6ab931ef..5616d2afca45 100644 > > --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt > > @@ -18,7 +18,7 @@ Messaging Unit Device Node: > > Required properties: > > ------------------- > > - compatible : should be "fsl,<chip>-mu", the supported chips include > > - imx8qxp, imx8qm. > > + imx6sx, imx7s, imx8qxp, imx8qm. > > > This is not scalable. Do we add every new SoC that contains the same controller? > I think the controller name, if it has one or create a new one like > 'imx-mu', should be used here. > So, > compatible : should be "fsl,imx-mu" No, for all the reasons already correctly explained in this thread. > BTW, the driver doesn't even probe anything other than > 'fsl,imx6sx-mu', unlike what the binding says. This aspect should be documented here though. The documentation should clearly define what are valid combinations of compatibles. Rob
On Tue, Jul 31, 2018 at 3:52 AM, Rob Herring <robh@kernel.org> wrote: > On Thu, Jul 26, 2018 at 03:25:15PM +0530, Jassi Brar wrote: >> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel >> <o.rempel@pengutronix.de> wrote: >> > This are currently tested SoCs with imx-mailbox driver. >> > >> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >> > --- >> > Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > index 113d6ab931ef..5616d2afca45 100644 >> > --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > @@ -18,7 +18,7 @@ Messaging Unit Device Node: >> > Required properties: >> > ------------------- >> > - compatible : should be "fsl,<chip>-mu", the supported chips include >> > - imx8qxp, imx8qm. >> > + imx6sx, imx7s, imx8qxp, imx8qm. >> > >> This is not scalable. Do we add every new SoC that contains the same controller? >> I think the controller name, if it has one or create a new one like >> 'imx-mu', should be used here. >> So, >> compatible : should be "fsl,imx-mu" > > No, for all the reasons already correctly explained in this thread. > OK. I curious >> BTW, the driver doesn't even probe anything other than >> 'fsl,imx6sx-mu', unlike what the binding says. > > This aspect should be documented here though. The documentation should > clearly define what are valid combinations of compatibles. > > Rob
On Tue, Jul 31, 2018 at 3:52 AM, Rob Herring <robh@kernel.org> wrote: > On Thu, Jul 26, 2018 at 03:25:15PM +0530, Jassi Brar wrote: >> On Thu, Jul 26, 2018 at 12:23 PM, Oleksij Rempel >> <o.rempel@pengutronix.de> wrote: >> > This are currently tested SoCs with imx-mailbox driver. >> > >> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >> > --- >> > Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > index 113d6ab931ef..5616d2afca45 100644 >> > --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt >> > @@ -18,7 +18,7 @@ Messaging Unit Device Node: >> > Required properties: >> > ------------------- >> > - compatible : should be "fsl,<chip>-mu", the supported chips include >> > - imx8qxp, imx8qm. >> > + imx6sx, imx7s, imx8qxp, imx8qm. >> > >> This is not scalable. Do we add every new SoC that contains the same controller? >> I think the controller name, if it has one or create a new one like >> 'imx-mu', should be used here. >> So, >> compatible : should be "fsl,imx-mu" > > No, for all the reasons already correctly explained in this thread. > OK. I am curious to see what soc specific fixes we need in future. Thanks.
diff --git a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt index 113d6ab931ef..5616d2afca45 100644 --- a/Documentation/devicetree/bindings/mailbox/fsl,mu.txt +++ b/Documentation/devicetree/bindings/mailbox/fsl,mu.txt @@ -18,7 +18,7 @@ Messaging Unit Device Node: Required properties: ------------------- - compatible : should be "fsl,<chip>-mu", the supported chips include - imx8qxp, imx8qm. + imx6sx, imx7s, imx8qxp, imx8qm. - reg : Should contain the registers location and length - interrupts : Interrupt number. The interrupt specifier format depends on the interrupt controller parent.
This are currently tested SoCs with imx-mailbox driver. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- Documentation/devicetree/bindings/mailbox/fsl,mu.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)