* [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes
@ 2016-11-15 11:00 Bartosz Golaszewski
2016-11-16 9:17 ` Sekhar Nori
[not found] ` <1479207611-18028-1-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
0 siblings, 2 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2016-11-15 11:00 UTC (permalink / raw)
To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha,
Tomi Valkeinen, David Airlie, Laurent Pinchart,
Bartosz Golaszewski
Add the nodes for the MSTPRI configuration and DDR2/mDDR memory
controller drivers to da850.dtsi.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
v1 -> v2:
- moved the priority controller node above the cfgchip node
- renamed added nodes to better reflect their purpose
arch/arm/boot/dts/da850.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 1bb1f6d..412eec6 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -210,6 +210,10 @@
};
};
+ prictrl: priority-controller@14110 {
+ compatible = "ti,da850-mstpri";
+ reg = <0x14110 0x0c>;
+ };
cfgchip: chip-controller@1417c {
compatible = "ti,da830-cfgchip", "syscon", "simple-mfd";
reg = <0x1417c 0x14>;
@@ -451,4 +455,8 @@
1 0 0x68000000 0x00008000>;
status = "disabled";
};
+ memctrl: memory-controller@b0000000 {
+ compatible = "ti,da850-ddr-controller";
+ reg = <0xb0000000 0xe8>;
+ };
};
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes 2016-11-15 11:00 [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes Bartosz Golaszewski @ 2016-11-16 9:17 ` Sekhar Nori [not found] ` <1479207611-18028-1-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 1 sibling, 0 replies; 10+ messages in thread From: Sekhar Nori @ 2016-11-16 9:17 UTC (permalink / raw) To: Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart On Tuesday 15 November 2016 04:30 PM, Bartosz Golaszewski wrote: > Add the nodes for the MSTPRI configuration and DDR2/mDDR memory > controller drivers to da850.dtsi. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> Applied to v4.10/dt Thanks, Sekhar ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <1479207611-18028-1-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes [not found] ` <1479207611-18028-1-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> @ 2016-11-22 22:23 ` David Lechner 2016-11-23 10:27 ` Bartosz Golaszewski 0 siblings, 1 reply; 10+ messages in thread From: David Lechner @ 2016-11-22 22:23 UTC (permalink / raw) To: Bartosz Golaszewski, Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King Cc: linux-devicetree, David Airlie, LKML, linux-drm, Tomi Valkeinen, Jyri Sarha, arm-soc, Laurent Pinchart On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote: > Add the nodes for the MSTPRI configuration and DDR2/mDDR memory > controller drivers to da850.dtsi. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> > --- > v1 -> v2: > - moved the priority controller node above the cfgchip node > - renamed added nodes to better reflect their purpose > > arch/arm/boot/dts/da850.dtsi | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi > index 1bb1f6d..412eec6 100644 > --- a/arch/arm/boot/dts/da850.dtsi > +++ b/arch/arm/boot/dts/da850.dtsi > @@ -210,6 +210,10 @@ > }; > > }; > + prictrl: priority-controller@14110 { > + compatible = "ti,da850-mstpri"; > + reg = <0x14110 0x0c>; I think we should add status = "disabled"; here and let boards opt in. > + }; > cfgchip: chip-controller@1417c { > compatible = "ti,da830-cfgchip", "syscon", "simple-mfd"; > reg = <0x1417c 0x14>; > @@ -451,4 +455,8 @@ > 1 0 0x68000000 0x00008000>; > status = "disabled"; > }; > + memctrl: memory-controller@b0000000 { > + compatible = "ti,da850-ddr-controller"; > + reg = <0xb0000000 0xe8>; same here. status = "disabled"; > + }; > }; > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes 2016-11-22 22:23 ` David Lechner @ 2016-11-23 10:27 ` Bartosz Golaszewski [not found] ` <CAMpxmJVqog0Fno_DO0NQKmVNvzu5d=3GOLk145wBmB-V70BLeg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Bartosz Golaszewski @ 2016-11-23 10:27 UTC (permalink / raw) To: David Lechner Cc: Mark Rutland, linux-devicetree, Tomi Valkeinen, Kevin Hilman, Michael Turquette, Sekhar Nori, Russell King, linux-drm, LKML, Peter Ujfalusi, Rob Herring, Jyri Sarha, Frank Rowand, arm-soc, Laurent Pinchart 2016-11-22 23:23 GMT+01:00 David Lechner <david@lechnology.com>: > On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote: >> >> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory >> controller drivers to da850.dtsi. >> >> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >> --- >> v1 -> v2: >> - moved the priority controller node above the cfgchip node >> - renamed added nodes to better reflect their purpose >> >> arch/arm/boot/dts/da850.dtsi | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi >> index 1bb1f6d..412eec6 100644 >> --- a/arch/arm/boot/dts/da850.dtsi >> +++ b/arch/arm/boot/dts/da850.dtsi >> @@ -210,6 +210,10 @@ >> }; >> >> }; >> + prictrl: priority-controller@14110 { >> + compatible = "ti,da850-mstpri"; >> + reg = <0x14110 0x0c>; > > > I think we should add status = "disabled"; here and let boards opt in. > >> + }; >> cfgchip: chip-controller@1417c { >> compatible = "ti,da830-cfgchip", "syscon", >> "simple-mfd"; >> reg = <0x1417c 0x14>; >> @@ -451,4 +455,8 @@ >> 1 0 0x68000000 0x00008000>; >> status = "disabled"; >> }; >> + memctrl: memory-controller@b0000000 { >> + compatible = "ti,da850-ddr-controller"; >> + reg = <0xb0000000 0xe8>; > > > same here. status = "disabled"; > >> + }; >> }; >> Hi David, I did that initially[1][2] and it was rejected by Kevin[3] and Laurent[4]. FYI this patch has already been queued by Sekhar. Best regards, Bartosz Golaszewski [1] https://www.spinics.net/lists/arm-kernel/msg539638.html [2] http://www.spinics.net/lists/devicetree/msg148575.html [3] http://www.spinics.net/lists/devicetree/msg148667.html [4] http://www.spinics.net/lists/devicetree/msg148655.html _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <CAMpxmJVqog0Fno_DO0NQKmVNvzu5d=3GOLk145wBmB-V70BLeg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes [not found] ` <CAMpxmJVqog0Fno_DO0NQKmVNvzu5d=3GOLk145wBmB-V70BLeg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2016-11-23 16:19 ` David Lechner 2016-11-23 22:32 ` Kevin Hilman 0 siblings, 1 reply; 10+ messages in thread From: David Lechner @ 2016-11-23 16:19 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King, linux-devicetree, David Airlie, LKML, linux-drm, Tomi Valkeinen, Jyri Sarha, arm-soc, Laurent Pinchart On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote: > 2016-11-22 23:23 GMT+01:00 David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>: >> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote: >>> >>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory >>> controller drivers to da850.dtsi. >>> >>> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>> --- >>> v1 -> v2: >>> - moved the priority controller node above the cfgchip node >>> - renamed added nodes to better reflect their purpose >>> >>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi >>> index 1bb1f6d..412eec6 100644 >>> --- a/arch/arm/boot/dts/da850.dtsi >>> +++ b/arch/arm/boot/dts/da850.dtsi >>> @@ -210,6 +210,10 @@ >>> }; >>> >>> }; >>> + prictrl: priority-controller@14110 { >>> + compatible = "ti,da850-mstpri"; >>> + reg = <0x14110 0x0c>; >> >> >> I think we should add status = "disabled"; here and let boards opt in. >> >>> + }; >>> cfgchip: chip-controller@1417c { >>> compatible = "ti,da830-cfgchip", "syscon", >>> "simple-mfd"; >>> reg = <0x1417c 0x14>; >>> @@ -451,4 +455,8 @@ >>> 1 0 0x68000000 0x00008000>; >>> status = "disabled"; >>> }; >>> + memctrl: memory-controller@b0000000 { >>> + compatible = "ti,da850-ddr-controller"; >>> + reg = <0xb0000000 0xe8>; >> >> >> same here. status = "disabled"; >> >>> + }; >>> }; >>> > > Hi David, > > I did that initially[1][2] and it was rejected by Kevin[3] and Laurent[4]. > > FYI this patch has already been queued by Sekhar. Thanks. I did not see those threads. FYI to maintainers, having these enabled by default causes error messages in the kernel log for other boards that are not supported by the drivers. Since there is only one board that is supported and soon to be 2 that are not, I would rather have this disabled by default to avoid the error messages. > > Best regards, > Bartosz Golaszewski > > [1] https://www.spinics.net/lists/arm-kernel/msg539638.html > [2] http://www.spinics.net/lists/devicetree/msg148575.html > [3] http://www.spinics.net/lists/devicetree/msg148667.html > [4] http://www.spinics.net/lists/devicetree/msg148655.html > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes 2016-11-23 16:19 ` David Lechner @ 2016-11-23 22:32 ` Kevin Hilman [not found] ` <m2bmx5x1y2.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Kevin Hilman @ 2016-11-23 22:32 UTC (permalink / raw) To: David Lechner Cc: Mark Rutland, linux-devicetree, Tomi Valkeinen, Michael Turquette, Sekhar Nori, Russell King, linux-drm, LKML, Peter Ujfalusi, Bartosz Golaszewski, Rob Herring, Jyri Sarha, Frank Rowand, arm-soc, Laurent Pinchart David Lechner <david@lechnology.com> writes: > On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote: >> 2016-11-22 23:23 GMT+01:00 David Lechner <david@lechnology.com>: >>> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote: >>>> >>>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory >>>> controller drivers to da850.dtsi. >>>> >>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>>> --- >>>> v1 -> v2: >>>> - moved the priority controller node above the cfgchip node >>>> - renamed added nodes to better reflect their purpose >>>> >>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++ >>>> 1 file changed, 8 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi >>>> index 1bb1f6d..412eec6 100644 >>>> --- a/arch/arm/boot/dts/da850.dtsi >>>> +++ b/arch/arm/boot/dts/da850.dtsi >>>> @@ -210,6 +210,10 @@ >>>> }; >>>> >>>> }; >>>> + prictrl: priority-controller@14110 { >>>> + compatible = "ti,da850-mstpri"; >>>> + reg = <0x14110 0x0c>; >>> >>> >>> I think we should add status = "disabled"; here and let boards opt in. >>> >>>> + }; >>>> cfgchip: chip-controller@1417c { >>>> compatible = "ti,da830-cfgchip", "syscon", >>>> "simple-mfd"; >>>> reg = <0x1417c 0x14>; >>>> @@ -451,4 +455,8 @@ >>>> 1 0 0x68000000 0x00008000>; >>>> status = "disabled"; >>>> }; >>>> + memctrl: memory-controller@b0000000 { >>>> + compatible = "ti,da850-ddr-controller"; >>>> + reg = <0xb0000000 0xe8>; >>> >>> >>> same here. status = "disabled"; >>> >>>> + }; >>>> }; >>>> >> >> Hi David, >> >> I did that initially[1][2] and it was rejected by Kevin[3] and Laurent[4]. >> >> FYI this patch has already been queued by Sekhar. > > Thanks. I did not see those threads. > > FYI to maintainers, having these enabled by default causes error > messages in the kernel log for other boards that are not supported by > the drivers. Then the driver is too noisy and should be cleaned up. > Since there is only one board that is supported and soon > to be 2 that are not, I would rather have this disabled by default to > avoid the error messages. IMO, what exactly are the error messages? Sounds like the driver is being too verbose, and calling things errors that are not really errors. Kevin _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <m2bmx5x1y2.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes [not found] ` <m2bmx5x1y2.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> @ 2016-11-23 22:48 ` David Lechner [not found] ` <f88df859-3c0f-38f2-275d-ce6a9996fb6e-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: David Lechner @ 2016-11-23 22:48 UTC (permalink / raw) To: Kevin Hilman Cc: Bartosz Golaszewski, Michael Turquette, Sekhar Nori, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King, linux-devicetree, David Airlie, LKML, linux-drm, Tomi Valkeinen, Jyri Sarha, arm-soc, Laurent Pinchart On 11/23/2016 04:32 PM, Kevin Hilman wrote: > David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> writes: > >> On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote: >>> 2016-11-22 23:23 GMT+01:00 David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>: >>>> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote: >>>>> >>>>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory >>>>> controller drivers to da850.dtsi. >>>>> >>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>>>> --- >>>>> v1 -> v2: >>>>> - moved the priority controller node above the cfgchip node >>>>> - renamed added nodes to better reflect their purpose >>>>> >>>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++ >>>>> 1 file changed, 8 insertions(+) >>>>> >>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi >>>>> index 1bb1f6d..412eec6 100644 >>>>> --- a/arch/arm/boot/dts/da850.dtsi >>>>> +++ b/arch/arm/boot/dts/da850.dtsi >>>>> @@ -210,6 +210,10 @@ >>>>> }; >>>>> >>>>> }; >>>>> + prictrl: priority-controller@14110 { >>>>> + compatible = "ti,da850-mstpri"; >>>>> + reg = <0x14110 0x0c>; >>>> >>>> >>>> I think we should add status = "disabled"; here and let boards opt in. >>>> >>>>> + }; >>>>> cfgchip: chip-controller@1417c { >>>>> compatible = "ti,da830-cfgchip", "syscon", >>>>> "simple-mfd"; >>>>> reg = <0x1417c 0x14>; >>>>> @@ -451,4 +455,8 @@ >>>>> 1 0 0x68000000 0x00008000>; >>>>> status = "disabled"; >>>>> }; >>>>> + memctrl: memory-controller@b0000000 { >>>>> + compatible = "ti,da850-ddr-controller"; >>>>> + reg = <0xb0000000 0xe8>; >>>> >>>> >>>> same here. status = "disabled"; >>>> >>>>> + }; >>>>> }; >>>>> >>> >>> Hi David, >>> >>> I did that initially[1][2] and it was rejected by Kevin[3] and Laurent[4]. >>> >>> FYI this patch has already been queued by Sekhar. >> >> Thanks. I did not see those threads. >> >> FYI to maintainers, having these enabled by default causes error >> messages in the kernel log for other boards that are not supported by >> the drivers. > > Then the driver is too noisy and should be cleaned up. > >> Since there is only one board that is supported and soon >> to be 2 that are not, I would rather have this disabled by default to >> avoid the error messages. > > IMO, what exactly are the error messages? Sounds like the driver is > being too verbose, and calling things errors that are not really errors. It is just one line per driver. dev_err(dev, "no master priorities defined for this board\n"); and dev_err(dev, "no settings defined for this board\n"); Since "ti,da850-lcdk" is the only board supported in these drivers, all other boards will see these error messages. Also, these modules will be loaded and taking up memory on boards that don't use them. This not really a big deal and they can be explicitly disabled, so maybe it was not worth mentioning. > > Kevin > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <f88df859-3c0f-38f2-275d-ce6a9996fb6e-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>]
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes [not found] ` <f88df859-3c0f-38f2-275d-ce6a9996fb6e-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> @ 2016-11-24 5:03 ` Sekhar Nori [not found] ` <20fbc946-d56c-31a3-4ae7-cf61df96a3c3-l0cyMroinI0@public.gmane.org> 2016-11-28 6:16 ` Kevin Hilman 0 siblings, 2 replies; 10+ messages in thread From: Sekhar Nori @ 2016-11-24 5:03 UTC (permalink / raw) To: David Lechner, Kevin Hilman Cc: Bartosz Golaszewski, Michael Turquette, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King, linux-devicetree, David Airlie, LKML, linux-drm, Tomi Valkeinen, Jyri Sarha, arm-soc, Laurent Pinchart On Thursday 24 November 2016 04:18 AM, David Lechner wrote: > On 11/23/2016 04:32 PM, Kevin Hilman wrote: >> David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> writes: >> >>> On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote: >>>> 2016-11-22 23:23 GMT+01:00 David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>: >>>>> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote: >>>>>> >>>>>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory >>>>>> controller drivers to da850.dtsi. >>>>>> >>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>>>>> --- >>>>>> v1 -> v2: >>>>>> - moved the priority controller node above the cfgchip node >>>>>> - renamed added nodes to better reflect their purpose >>>>>> >>>>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++ >>>>>> 1 file changed, 8 insertions(+) >>>>>> >>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi >>>>>> b/arch/arm/boot/dts/da850.dtsi >>>>>> index 1bb1f6d..412eec6 100644 >>>>>> --- a/arch/arm/boot/dts/da850.dtsi >>>>>> +++ b/arch/arm/boot/dts/da850.dtsi >>>>>> @@ -210,6 +210,10 @@ >>>>>> }; >>>>>> >>>>>> }; >>>>>> + prictrl: priority-controller@14110 { >>>>>> + compatible = "ti,da850-mstpri"; >>>>>> + reg = <0x14110 0x0c>; >>>>> >>>>> >>>>> I think we should add status = "disabled"; here and let boards opt in. >>>>> >>>>>> + }; >>>>>> cfgchip: chip-controller@1417c { >>>>>> compatible = "ti,da830-cfgchip", "syscon", >>>>>> "simple-mfd"; >>>>>> reg = <0x1417c 0x14>; >>>>>> @@ -451,4 +455,8 @@ >>>>>> 1 0 0x68000000 0x00008000>; >>>>>> status = "disabled"; >>>>>> }; >>>>>> + memctrl: memory-controller@b0000000 { >>>>>> + compatible = "ti,da850-ddr-controller"; >>>>>> + reg = <0xb0000000 0xe8>; >>>>> >>>>> >>>>> same here. status = "disabled"; >>>>> >>>>>> + }; >>>>>> }; >>>>>> >>>> >>>> Hi David, >>>> >>>> I did that initially[1][2] and it was rejected by Kevin[3] and >>>> Laurent[4]. >>>> >>>> FYI this patch has already been queued by Sekhar. >>> >>> Thanks. I did not see those threads. >>> >>> FYI to maintainers, having these enabled by default causes error >>> messages in the kernel log for other boards that are not supported by >>> the drivers. >> >> Then the driver is too noisy and should be cleaned up. >> >>> Since there is only one board that is supported and soon >>> to be 2 that are not, I would rather have this disabled by default to >>> avoid the error messages. >> >> IMO, what exactly are the error messages? Sounds like the driver is >> being too verbose, and calling things errors that are not really errors. > > It is just one line per driver. > > dev_err(dev, "no master priorities defined for this board\n"); > > and > > dev_err(dev, "no settings defined for this board\n"); > > > Since "ti,da850-lcdk" is the only board supported in these drivers, all > other boards will see these error messages. Thats pretty bad. Sorry about that. The original justification for keeping them enabled all the time was that they are in-SoC modules with no external dependencies (like IO lines or voltage rails) so they can be enabled on all boards that use DA850. While that remains true, the configuration itself is board specific. I think the error messages are still useful, so instead of silencing them, I think we should go back to keeping these nodes disabled by default and enabling only on boards which have support for it in the driver. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20fbc946-d56c-31a3-4ae7-cf61df96a3c3-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes [not found] ` <20fbc946-d56c-31a3-4ae7-cf61df96a3c3-l0cyMroinI0@public.gmane.org> @ 2016-11-24 8:48 ` Bartosz Golaszewski 0 siblings, 0 replies; 10+ messages in thread From: Bartosz Golaszewski @ 2016-11-24 8:48 UTC (permalink / raw) To: Sekhar Nori Cc: David Lechner, Kevin Hilman, Michael Turquette, Rob Herring, Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King, linux-devicetree, David Airlie, LKML, linux-drm, Tomi Valkeinen, Jyri Sarha, arm-soc, Laurent Pinchart 2016-11-24 6:03 GMT+01:00 Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>: > On Thursday 24 November 2016 04:18 AM, David Lechner wrote: >> On 11/23/2016 04:32 PM, Kevin Hilman wrote: >>> David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org> writes: >>> >>>> On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote: >>>>> 2016-11-22 23:23 GMT+01:00 David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>: >>>>>> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote: >>>>>>> >>>>>>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory >>>>>>> controller drivers to da850.dtsi. >>>>>>> >>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> >>>>>>> --- >>>>>>> v1 -> v2: >>>>>>> - moved the priority controller node above the cfgchip node >>>>>>> - renamed added nodes to better reflect their purpose >>>>>>> >>>>>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++ >>>>>>> 1 file changed, 8 insertions(+) >>>>>>> >>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi >>>>>>> b/arch/arm/boot/dts/da850.dtsi >>>>>>> index 1bb1f6d..412eec6 100644 >>>>>>> --- a/arch/arm/boot/dts/da850.dtsi >>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi >>>>>>> @@ -210,6 +210,10 @@ >>>>>>> }; >>>>>>> >>>>>>> }; >>>>>>> + prictrl: priority-controller@14110 { >>>>>>> + compatible = "ti,da850-mstpri"; >>>>>>> + reg = <0x14110 0x0c>; >>>>>> >>>>>> >>>>>> I think we should add status = "disabled"; here and let boards opt in. >>>>>> >>>>>>> + }; >>>>>>> cfgchip: chip-controller@1417c { >>>>>>> compatible = "ti,da830-cfgchip", "syscon", >>>>>>> "simple-mfd"; >>>>>>> reg = <0x1417c 0x14>; >>>>>>> @@ -451,4 +455,8 @@ >>>>>>> 1 0 0x68000000 0x00008000>; >>>>>>> status = "disabled"; >>>>>>> }; >>>>>>> + memctrl: memory-controller@b0000000 { >>>>>>> + compatible = "ti,da850-ddr-controller"; >>>>>>> + reg = <0xb0000000 0xe8>; >>>>>> >>>>>> >>>>>> same here. status = "disabled"; >>>>>> >>>>>>> + }; >>>>>>> }; >>>>>>> >>>>> >>>>> Hi David, >>>>> >>>>> I did that initially[1][2] and it was rejected by Kevin[3] and >>>>> Laurent[4]. >>>>> >>>>> FYI this patch has already been queued by Sekhar. >>>> >>>> Thanks. I did not see those threads. >>>> >>>> FYI to maintainers, having these enabled by default causes error >>>> messages in the kernel log for other boards that are not supported by >>>> the drivers. >>> >>> Then the driver is too noisy and should be cleaned up. >>> >>>> Since there is only one board that is supported and soon >>>> to be 2 that are not, I would rather have this disabled by default to >>>> avoid the error messages. >>> >>> IMO, what exactly are the error messages? Sounds like the driver is >>> being too verbose, and calling things errors that are not really errors. >> >> It is just one line per driver. >> >> dev_err(dev, "no master priorities defined for this board\n"); >> >> and >> >> dev_err(dev, "no settings defined for this board\n"); >> >> >> Since "ti,da850-lcdk" is the only board supported in these drivers, all >> other boards will see these error messages. > > Thats pretty bad. Sorry about that. The original justification for > keeping them enabled all the time was that they are in-SoC modules with > no external dependencies (like IO lines or voltage rails) so they can be > enabled on all boards that use DA850. While that remains true, the > configuration itself is board specific. > > I think the error messages are still useful, so instead of silencing > them, I think we should go back to keeping these nodes disabled by > default and enabling only on boards which have support for it in the driver. > > Thanks, > Sekhar I'll send a patch. Thanks, Bartosz -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes 2016-11-24 5:03 ` Sekhar Nori [not found] ` <20fbc946-d56c-31a3-4ae7-cf61df96a3c3-l0cyMroinI0@public.gmane.org> @ 2016-11-28 6:16 ` Kevin Hilman 1 sibling, 0 replies; 10+ messages in thread From: Kevin Hilman @ 2016-11-28 6:16 UTC (permalink / raw) To: Sekhar Nori Cc: Mark Rutland, linux-devicetree, David Lechner, Tomi Valkeinen, Michael Turquette, Russell King, linux-drm, LKML, Peter Ujfalusi, Bartosz Golaszewski, Rob Herring, Jyri Sarha, Frank Rowand, arm-soc, Laurent Pinchart On Wed, Nov 23, 2016 at 9:03 PM, Sekhar Nori <nsekhar@ti.com> wrote: > On Thursday 24 November 2016 04:18 AM, David Lechner wrote: >> On 11/23/2016 04:32 PM, Kevin Hilman wrote: >>> David Lechner <david@lechnology.com> writes: >>> >>>> On 11/23/2016 04:27 AM, Bartosz Golaszewski wrote: >>>>> 2016-11-22 23:23 GMT+01:00 David Lechner <david@lechnology.com>: >>>>>> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote: >>>>>>> >>>>>>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory >>>>>>> controller drivers to da850.dtsi. >>>>>>> >>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>>>>>> --- >>>>>>> v1 -> v2: >>>>>>> - moved the priority controller node above the cfgchip node >>>>>>> - renamed added nodes to better reflect their purpose >>>>>>> >>>>>>> arch/arm/boot/dts/da850.dtsi | 8 ++++++++ >>>>>>> 1 file changed, 8 insertions(+) >>>>>>> >>>>>>> diff --git a/arch/arm/boot/dts/da850.dtsi >>>>>>> b/arch/arm/boot/dts/da850.dtsi >>>>>>> index 1bb1f6d..412eec6 100644 >>>>>>> --- a/arch/arm/boot/dts/da850.dtsi >>>>>>> +++ b/arch/arm/boot/dts/da850.dtsi >>>>>>> @@ -210,6 +210,10 @@ >>>>>>> }; >>>>>>> >>>>>>> }; >>>>>>> + prictrl: priority-controller@14110 { >>>>>>> + compatible = "ti,da850-mstpri"; >>>>>>> + reg = <0x14110 0x0c>; >>>>>> >>>>>> >>>>>> I think we should add status = "disabled"; here and let boards opt in. >>>>>> >>>>>>> + }; >>>>>>> cfgchip: chip-controller@1417c { >>>>>>> compatible = "ti,da830-cfgchip", "syscon", >>>>>>> "simple-mfd"; >>>>>>> reg = <0x1417c 0x14>; >>>>>>> @@ -451,4 +455,8 @@ >>>>>>> 1 0 0x68000000 0x00008000>; >>>>>>> status = "disabled"; >>>>>>> }; >>>>>>> + memctrl: memory-controller@b0000000 { >>>>>>> + compatible = "ti,da850-ddr-controller"; >>>>>>> + reg = <0xb0000000 0xe8>; >>>>>> >>>>>> >>>>>> same here. status = "disabled"; >>>>>> >>>>>>> + }; >>>>>>> }; >>>>>>> >>>>> >>>>> Hi David, >>>>> >>>>> I did that initially[1][2] and it was rejected by Kevin[3] and >>>>> Laurent[4]. >>>>> >>>>> FYI this patch has already been queued by Sekhar. >>>> >>>> Thanks. I did not see those threads. >>>> >>>> FYI to maintainers, having these enabled by default causes error >>>> messages in the kernel log for other boards that are not supported by >>>> the drivers. >>> >>> Then the driver is too noisy and should be cleaned up. >>> >>>> Since there is only one board that is supported and soon >>>> to be 2 that are not, I would rather have this disabled by default to >>>> avoid the error messages. >>> >>> IMO, what exactly are the error messages? Sounds like the driver is >>> being too verbose, and calling things errors that are not really errors. >> >> It is just one line per driver. >> >> dev_err(dev, "no master priorities defined for this board\n"); >> >> and >> >> dev_err(dev, "no settings defined for this board\n"); >> >> >> Since "ti,da850-lcdk" is the only board supported in these drivers, all >> other boards will see these error messages. > > Thats pretty bad. Sorry about that. The original justification for > keeping them enabled all the time was that they are in-SoC modules with > no external dependencies (like IO lines or voltage rails) so they can be > enabled on all boards that use DA850. While that remains true, the > configuration itself is board specific. > > I think the error messages are still useful, so instead of silencing > them, I think we should go back to keeping these nodes disabled by > default and enabling only on boards which have support for it in the driver. I don't have a strong preference for the enabled/disabled by default, but I think the error messages are not error messages. It seems perfectly reasonable for boards to accept the reset (or bootloader) configuration of these registers, and not consider that an error. Kevin _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-11-28 6:16 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-15 11:00 [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes Bartosz Golaszewski
2016-11-16 9:17 ` Sekhar Nori
[not found] ` <1479207611-18028-1-git-send-email-bgolaszewski-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
2016-11-22 22:23 ` David Lechner
2016-11-23 10:27 ` Bartosz Golaszewski
[not found] ` <CAMpxmJVqog0Fno_DO0NQKmVNvzu5d=3GOLk145wBmB-V70BLeg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-11-23 16:19 ` David Lechner
2016-11-23 22:32 ` Kevin Hilman
[not found] ` <m2bmx5x1y2.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
2016-11-23 22:48 ` David Lechner
[not found] ` <f88df859-3c0f-38f2-275d-ce6a9996fb6e-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
2016-11-24 5:03 ` Sekhar Nori
[not found] ` <20fbc946-d56c-31a3-4ae7-cf61df96a3c3-l0cyMroinI0@public.gmane.org>
2016-11-24 8:48 ` Bartosz Golaszewski
2016-11-28 6:16 ` Kevin Hilman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).