| Message ID | 20180928182954.25446-3-dmurphy@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TI LMU and Dedicated Drivers | expand |
On Fri 2018-09-28 13:29:47, Dan Murphy wrote: > Remove support for the LM3697 LED device > from the ti-lmu. The LM3697 will be supported > via a stand alone LED driver. > > Signed-off-by: Dan Murphy <dmurphy@ti.com> NAK, for reasons I explained before. Please add it to the patch so that it does not get applied by mistake. Ouch and AFAICT Rob was not happy with this either. Yes, you are creating new drivers, ok; but that does _not_ mean you should create new binding. Pavel
Hello On 10/02/2018 02:28 AM, Pavel Machek wrote: > On Fri 2018-09-28 13:29:47, Dan Murphy wrote: >> Remove support for the LM3697 LED device >> from the ti-lmu. The LM3697 will be supported >> via a stand alone LED driver. >> >> Signed-off-by: Dan Murphy <dmurphy@ti.com> > > NAK, for reasons I explained before. Please add it to the patch so > that it does not get applied by mistake. Ouch and AFAICT Rob was not > happy with this either. > > Yes, you are creating new drivers, ok; but that does _not_ mean you > should create new binding. I am copying my comment here on the review of this original binding for records I found the review or at least the reference for the ti-lmu.txt binding. https://lore.kernel.org/patchwork/patch/764180/ Does not appear that the binding was sent to the device tree mail list. (Maybe that email list did not exist in Feb 2017). Especially with the amount of change that is being submitted in the newer patchsets. <new content> Having found this submission and no comments on the review I would think we need to take an exception on these bindings. Dan > Pavel >
On Wed 2018-10-03 07:24:23, Dan Murphy wrote: > Hello > > On 10/02/2018 02:28 AM, Pavel Machek wrote: > > On Fri 2018-09-28 13:29:47, Dan Murphy wrote: > >> Remove support for the LM3697 LED device > >> from the ti-lmu. The LM3697 will be supported > >> via a stand alone LED driver. > >> > >> Signed-off-by: Dan Murphy <dmurphy@ti.com> > > > > NAK, for reasons I explained before. Please add it to the patch so > > that it does not get applied by mistake. Ouch and AFAICT Rob was not > > happy with this either. > > > > Yes, you are creating new drivers, ok; but that does _not_ mean you > > should create new binding. > > I am copying my comment here on the review of this original binding for > records > > I found the review or at least the reference for the ti-lmu.txt binding. > > https://lore.kernel.org/patchwork/patch/764180/ > > Does not appear that the binding was sent to the device tree mail list. > (Maybe that email list did not exist in Feb 2017). Quick google shows: https://lwn.net/Articles/666023/ Now can we stop this nonsense? If there is a problem with the binding, submit patches to fix the problem. Pavel
On 10/03/2018 03:01 PM, Pavel Machek wrote: > On Wed 2018-10-03 07:24:23, Dan Murphy wrote: >> Hello >> >> On 10/02/2018 02:28 AM, Pavel Machek wrote: >>> On Fri 2018-09-28 13:29:47, Dan Murphy wrote: >>>> Remove support for the LM3697 LED device >>>> from the ti-lmu. The LM3697 will be supported >>>> via a stand alone LED driver. >>>> >>>> Signed-off-by: Dan Murphy <dmurphy@ti.com> >>> >>> NAK, for reasons I explained before. Please add it to the patch so >>> that it does not get applied by mistake. Ouch and AFAICT Rob was not >>> happy with this either. >>> >>> Yes, you are creating new drivers, ok; but that does _not_ mean you >>> should create new binding. >> >> I am copying my comment here on the review of this original binding for >> records >> >> I found the review or at least the reference for the ti-lmu.txt binding. >> >> https://lore.kernel.org/patchwork/patch/764180/ >> >> Does not appear that the binding was sent to the device tree mail list. >> (Maybe that email list did not exist in Feb 2017). > > Quick google shows: > > https://lwn.net/Articles/666023/ This link refreshed my memory and allowed me to recall Milo's patch set from the end of 2015, and the related discussion. I had an impression that I had had some request regarding the bindings but there was no follow-up. I've googled that thread [0] and it proved I was right [1]. From Milo's messages we can infer that there will be next version of the patch set and it appeared but over a year later [2], and without the leds-lm3633 driver and related bindings, and without drivers/video/backlight/ti-lmu-backlight-core.c. Patch set gets merged despite dangling DT references. Dan, I propose you to resend the patch removing the bindings from MFD, and explain the rationale in the patch below commit message after "---" . > Now can we stop this nonsense? If there is a problem with the binding, > submit patches to fix the problem. [0] https://lore.kernel.org/lkml/1448521025-2796-1-git-send-email-milo.kim@ti.com/ [1] https://lore.kernel.org/lkml/56656468.8020300@samsung.com/ [2] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1341860.html
Jacek On 10/03/2018 03:46 PM, Jacek Anaszewski wrote: > On 10/03/2018 03:01 PM, Pavel Machek wrote: >> On Wed 2018-10-03 07:24:23, Dan Murphy wrote: >>> Hello >>> >>> On 10/02/2018 02:28 AM, Pavel Machek wrote: >>>> On Fri 2018-09-28 13:29:47, Dan Murphy wrote: >>>>> Remove support for the LM3697 LED device >>>>> from the ti-lmu. The LM3697 will be supported >>>>> via a stand alone LED driver. >>>>> >>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com> >>>> >>>> NAK, for reasons I explained before. Please add it to the patch so >>>> that it does not get applied by mistake. Ouch and AFAICT Rob was not >>>> happy with this either. >>>> >>>> Yes, you are creating new drivers, ok; but that does _not_ mean you >>>> should create new binding. >>> >>> I am copying my comment here on the review of this original binding for >>> records >>> >>> I found the review or at least the reference for the ti-lmu.txt binding. >>> >>> https://lore.kernel.org/patchwork/patch/764180/ >>> >>> Does not appear that the binding was sent to the device tree mail list. >>> (Maybe that email list did not exist in Feb 2017). >> >> Quick google shows: >> >> https://lwn.net/Articles/666023/ > > This link refreshed my memory and allowed me to recall Milo's patch set > from the end of 2015, and the related discussion. I had an impression > that I had had some request regarding the bindings but there was no > follow-up. > > I've googled that thread [0] and it proved I was right [1]. > > From Milo's messages we can infer that there will be next > version of the patch set and it appeared but over a year later [2], > and without the leds-lm3633 driver and related bindings, and without > drivers/video/backlight/ti-lmu-backlight-core.c. > > Patch set gets merged despite dangling DT references. > > Dan, I propose you to resend the patch removing the bindings from > MFD, and explain the rationale in the patch below commit message > after "---" . > >> Now can we stop this nonsense? If there is a problem with the binding, >> submit patches to fix the problem. > > [0] > https://lore.kernel.org/lkml/1448521025-2796-1-git-send-email-milo.kim@ti.com/ > [1] https://lore.kernel.org/lkml/56656468.8020300@samsung.com/ > [2] > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1341860.html > Sounds good I will clean this up and submit a v3 non-RFC edition Dan
diff --git a/Documentation/devicetree/bindings/mfd/ti-lmu.txt b/Documentation/devicetree/bindings/mfd/ti-lmu.txt index c885cf89b8ce..920f910be4e9 100644 --- a/Documentation/devicetree/bindings/mfd/ti-lmu.txt +++ b/Documentation/devicetree/bindings/mfd/ti-lmu.txt @@ -9,7 +9,6 @@ TI LMU driver supports lighting devices below. LM3632 Backlight and regulator LM3633 Backlight, LED and fault monitor LM3695 Backlight - LM3697 Backlight and fault monitor Required properties: - compatible: Should be one of: @@ -18,11 +17,10 @@ Required properties: "ti,lm3632" "ti,lm3633" "ti,lm3695" - "ti,lm3697" - reg: I2C slave address. 0x11 for LM3632 0x29 for LM3631 - 0x36 for LM3633, LM3697 + 0x36 for LM3633 0x38 for LM3532 0x63 for LM3695 @@ -38,7 +36,6 @@ Optional nodes: Required properties: - compatible: Should be one of: "ti,lm3633-fault-monitor" - "ti,lm3697-fault-monitor" - leds: LED properties for LM3633. Please refer to [2]. - regulators: Regulator properties for LM3631 and LM3632. Please refer to [3]. @@ -220,24 +217,3 @@ lm3695@63 { }; }; }; - -lm3697@36 { - compatible = "ti,lm3697"; - reg = <0x36>; - - enable-gpios = <&pioC 2 GPIO_ACTIVE_HIGH>; - - backlight { - compatible = "ti,lm3697-backlight"; - - lcd { - led-sources = <0 1 2>; - ramp-up-msec = <200>; - ramp-down-msec = <200>; - }; - }; - - fault-monitor { - compatible = "ti,lm3697-fault-monitor"; - }; -};
Remove support for the LM3697 LED device from the ti-lmu. The LM3697 will be supported via a stand alone LED driver. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- .../devicetree/bindings/mfd/ti-lmu.txt | 26 +------------------ 1 file changed, 1 insertion(+), 25 deletions(-)