| Message ID | 1431005924-21777-7-git-send-email-lee.jones@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Lee, On Thu, 07 May 2015, Lee Jones wrote: > Signed-off-by: Lee Jones <lee.jones@linaro.org> > --- > Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt b/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt > index 388c88a..0b1a784 100644 > --- a/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt > +++ b/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt > @@ -1,20 +1,23 @@ > STMicroelectronics Low Power Controller (LPC) - Watchdog > ======================================================== > > -LPC currently supports Watchdog OR Real Time Clock functionality. > +LPC currently supports Watchdog OR Real Time Clock OR Clocksource > +functionality. > > [See: ../rtc/rtc-st-lpc.txt for RTC options] > +[See: ../timer/st,stih407-lpc for Clocksource options] > > Required properties > > -- compatible : Must be one of: "st,stih407-lpc" "st,stih416-lpc" > - "st,stih415-lpc" "st,stid127-lpc" > +- compatible : Must be one of: "st,stih407-lpc" The same comment as the RTC DT patch, you are removing the compatibles documentation for the other supported platforms like stih416-lpc. AFAIK they are required in the driver to get the correct sysconfig register. regards, Peter. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > --- > > Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt | 15 +++++++++------ > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt b/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt > > index 388c88a..0b1a784 100644 > > --- a/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt > > +++ b/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt > > @@ -1,20 +1,23 @@ > > STMicroelectronics Low Power Controller (LPC) - Watchdog > > ======================================================== > > > > -LPC currently supports Watchdog OR Real Time Clock functionality. > > +LPC currently supports Watchdog OR Real Time Clock OR Clocksource > > +functionality. > > > > [See: ../rtc/rtc-st-lpc.txt for RTC options] > > +[See: ../timer/st,stih407-lpc for Clocksource options] > > > > Required properties > > > > -- compatible : Must be one of: "st,stih407-lpc" "st,stih416-lpc" > > - "st,stih415-lpc" "st,stid127-lpc" > > +- compatible : Must be one of: "st,stih407-lpc" > > The same comment as the RTC DT patch, you are removing the compatibles > documentation for the other supported platforms like stih416-lpc. > AFAIK they are required in the driver to get the correct sysconfig register. That's intentional. I haven't yet tested any of this IP on STiH41{5,6} & STiH127. Due to lack of documentation, I'm not even sure if this IP even exists on some of the other platforms. I will add them back when support is added to both driver and DTB and I've been able to test them.
Hi Lee, On Thu, 07 May 2015, Lee Jones wrote: <snip> > > > Required properties > > > > > > -- compatible : Must be one of: "st,stih407-lpc" "st,stih416-lpc" > > > - "st,stih415-lpc" "st,stid127-lpc" > > > +- compatible : Must be one of: "st,stih407-lpc" > > > > The same comment as the RTC DT patch, you are removing the compatibles > > documentation for the other supported platforms like stih416-lpc. > > AFAIK they are required in the driver to get the correct sysconfig register. > > That's intentional. I haven't yet tested any of this IP on > STiH41{5,6} & STiH127. Due to lack of documentation, I'm not even > sure if this IP even exists on some of the other platforms. I will > add them back when support is added to both driver and DTB and I've > been able to test them. That was kind of my point, the driver code AFAIK already contains support for these SoC's. I would either expect the patch to remove support from the DT docs AND the driver, or leave it as is. It seems odd to only change the DT docs, and become unaligned to the code (this assumes I'm looking at the latest patchset here https://lkml.org/lkml/2015/3/4/1088 which includes support for these SoCs). regards, Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Thu, 07 May 2015, Peter Griffin wrote: > Hi Lee, > > On Thu, 07 May 2015, Lee Jones wrote: > > <snip> > > > > > Required properties > > > > > > > > -- compatible : Must be one of: "st,stih407-lpc" "st,stih416-lpc" > > > > - "st,stih415-lpc" "st,stid127-lpc" > > > > +- compatible : Must be one of: "st,stih407-lpc" > > > > > > The same comment as the RTC DT patch, you are removing the compatibles > > > documentation for the other supported platforms like stih416-lpc. > > > AFAIK they are required in the driver to get the correct sysconfig register. > > > > That's intentional. I haven't yet tested any of this IP on > > STiH41{5,6} & STiH127. Due to lack of documentation, I'm not even > > sure if this IP even exists on some of the other platforms. I will > > add them back when support is added to both driver and DTB and I've > > been able to test them. > > That was kind of my point, the driver code AFAIK already contains support > for these SoC's. > > I would either expect the patch to remove support from the DT docs AND the > driver, or leave it as is. > > It seems odd to only change the DT docs, and become unaligned to the > code (this assumes I'm looking at the latest patchset here > https://lkml.org/lkml/2015/3/4/1088 which includes support for these SoCs). The decision to remove these 'supported' platforms was made on the RTC side, where there is only support for "st,stih407-lpc" in the driver. I thought it best to mirror that thought over to the Watchdog LPC bindings, but thought it not really worth ripping out existing support from the driver. If others wish to test/use the Watchdog on other platforms and can read C code, they'll know what to do. Hopefully all of this will be a non-issue anyway, as I plan to test this on the other boards I have in my farm and make the necessary changes in the upcoming weeks. All should be squared away by the next merge-window.
Hi Lee, <snip> On Fri, 08 May 2015, Lee Jones wrote: > > > > > > > > > > -- compatible : Must be one of: "st,stih407-lpc" "st,stih416-lpc" > > > > > - "st,stih415-lpc" "st,stid127-lpc" > > > > > +- compatible : Must be one of: "st,stih407-lpc" > > > > > > > > The same comment as the RTC DT patch, you are removing the compatibles > > > > documentation for the other supported platforms like stih416-lpc. > > > > AFAIK they are required in the driver to get the correct sysconfig register. > > > > > > That's intentional. I haven't yet tested any of this IP on > > > STiH41{5,6} & STiH127. Due to lack of documentation, I'm not even > > > sure if this IP even exists on some of the other platforms. I will > > > add them back when support is added to both driver and DTB and I've > > > been able to test them. > > > > That was kind of my point, the driver code AFAIK already contains support > > for these SoC's. > > > > I would either expect the patch to remove support from the DT docs AND the > > driver, or leave it as is. > > > > It seems odd to only change the DT docs, and become unaligned to the > > code (this assumes I'm looking at the latest patchset here > > https://lkml.org/lkml/2015/3/4/1088 which includes support for these SoCs). > > The decision to remove these 'supported' platforms was made on the RTC > side, where there is only support for "st,stih407-lpc" in the driver. The change makes sense in the RTC case then. > I thought it best to mirror that thought over to the Watchdog LPC > bindings, Why? This is the bit I disagree with, the wdt DT docs IMHO should match the wdt driver code. > but thought it not really worth ripping out existing support > from the driver. I agree it is not worth ripping out existing support only to add it back in. Equally I can't see the point in doing this with the DT documentation either. > If others wish to test/use the Watchdog on other > platforms and can read C code, they'll know what to do. Yes, and without this change they could also read the documentation. Your doing a v2 anyway, it seems trivial to fix this at the same time. regards, Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 08 May 2015, Peter Griffin wrote: > On Fri, 08 May 2015, Lee Jones wrote: > > > > > > > > > > > > -- compatible : Must be one of: "st,stih407-lpc" "st,stih416-lpc" > > > > > > - "st,stih415-lpc" "st,stid127-lpc" > > > > > > +- compatible : Must be one of: "st,stih407-lpc" > > > > > > > > > > The same comment as the RTC DT patch, you are removing the compatibles > > > > > documentation for the other supported platforms like stih416-lpc. > > > > > AFAIK they are required in the driver to get the correct sysconfig register. > > > > > > > > That's intentional. I haven't yet tested any of this IP on > > > > STiH41{5,6} & STiH127. Due to lack of documentation, I'm not even > > > > sure if this IP even exists on some of the other platforms. I will > > > > add them back when support is added to both driver and DTB and I've > > > > been able to test them. > > > > > > That was kind of my point, the driver code AFAIK already contains support > > > for these SoC's. > > > > > > I would either expect the patch to remove support from the DT docs AND the > > > driver, or leave it as is. > > > > > > It seems odd to only change the DT docs, and become unaligned to the > > > code (this assumes I'm looking at the latest patchset here > > > https://lkml.org/lkml/2015/3/4/1088 which includes support for these SoCs). > > > > The decision to remove these 'supported' platforms was made on the RTC > > side, where there is only support for "st,stih407-lpc" in the driver. > > The change makes sense in the RTC case then. > > > I thought it best to mirror that thought over to the Watchdog LPC > > bindings, > > Why? This is the bit I disagree with, the wdt DT docs IMHO should match the wdt > driver code. Fair enough. I'll add the 'supported' compatibles back in.
diff --git a/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt b/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt index 388c88a..0b1a784 100644 --- a/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt +++ b/Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt @@ -1,20 +1,23 @@ STMicroelectronics Low Power Controller (LPC) - Watchdog ======================================================== -LPC currently supports Watchdog OR Real Time Clock functionality. +LPC currently supports Watchdog OR Real Time Clock OR Clocksource +functionality. [See: ../rtc/rtc-st-lpc.txt for RTC options] +[See: ../timer/st,stih407-lpc for Clocksource options] Required properties -- compatible : Must be one of: "st,stih407-lpc" "st,stih416-lpc" - "st,stih415-lpc" "st,stid127-lpc" +- compatible : Must be one of: "st,stih407-lpc" - reg : LPC registers base address + size - interrupts : LPC interrupt line number and associated flags - clocks : Clock used by LPC device (See: ../clock/clock-bindings.txt) -- st,lpc-mode : The LPC can run either one of two modes ST_LPC_MODE_RTC [0] or - ST_LPC_MODE_WDT [1]. One (and only one) mode must be - selected. +- st,lpc-mode : The LPC can run either one of three modes: + ST_LPC_MODE_RTC [0] + ST_LPC_MODE_WDT [1] + ST_LPC_MODE_CLKSRC [2] + One (and only one) mode must be selected. Required properties [watchdog mode]
Signed-off-by: Lee Jones <lee.jones@linaro.org> --- Documentation/devicetree/bindings/watchdog/st_lpc_wdt.txt | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)