[v6,1/3] dt-bindings: pwm: add MediaTek display PWM bindings
diff mbox

Message ID 1437380237-28961-2-git-send-email-yh.huang@mediatek.com
State New
Headers show

Commit Message

YH Huang July 20, 2015, 8:17 a.m. UTC
Document the device-tree binding of MediatTek display PWM.
The PWM has one channel to control the backlight brightness for display.
It supports MT8173 and MT6595.

Signed-off-by: YH Huang <yh.huang@mediatek.com>
---
 .../devicetree/bindings/pwm/pwm-mtk-disp.txt       | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt

Comments

Matthias Brugger July 24, 2015, 8:40 a.m. UTC | #1
On Monday, July 20, 2015 04:17:15 PM YH Huang wrote:
> Document the device-tree binding of MediatTek display PWM.
> The PWM has one channel to control the backlight brightness for display.
> It supports MT8173 and MT6595.
> 
> Signed-off-by: YH Huang <yh.huang@mediatek.com>
> ---
>  .../devicetree/bindings/pwm/pwm-mtk-disp.txt       | 42
> ++++++++++++++++++++++ 1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt new file mode
> 100644
> index 0000000..f8f59ba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> @@ -0,0 +1,42 @@
> +MediaTek display PWM controller
> +
> +Required properties:
> + - compatible: should be "mediatek,<name>-disp-pwm":
> +   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
> +   - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.

I had another look on the mt6589 datasheet and for me it doesn't look like as 
if this drivers is compatible to mt6589.

DISP_PWM_CON_0 offset 0x10 maps to interrupt enable register and 
DISP_PWM_CON_1 offset 0x14 maps to interrupt status register.

This looks wrong to me, as you use both registers to write clock divider and 
clock period.

Regarding that this is v6 of the patch set, I would propose that you just drop 
the compatible string for mt6589 or you implement the register offset on basis 
of the compatible string so that mt6589 can you the driver as well.

Best regards,
Matthias
Daniel Kurtz July 24, 2015, 9 a.m. UTC | #2
On Fri, Jul 24, 2015 at 4:40 PM, Matthias Brugger
<matthias.bgg@gmail.com> wrote:
> On Monday, July 20, 2015 04:17:15 PM YH Huang wrote:
>> Document the device-tree binding of MediatTek display PWM.
>> The PWM has one channel to control the backlight brightness for display.
>> It supports MT8173 and MT6595.
>>
>> Signed-off-by: YH Huang <yh.huang@mediatek.com>
>> ---
>>  .../devicetree/bindings/pwm/pwm-mtk-disp.txt       | 42
>> ++++++++++++++++++++++ 1 file changed, 42 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
>> b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt new file mode
>> 100644
>> index 0000000..f8f59ba
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
>> @@ -0,0 +1,42 @@
>> +MediaTek display PWM controller
>> +
>> +Required properties:
>> + - compatible: should be "mediatek,<name>-disp-pwm":
>> +   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
>> +   - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
>
> I had another look on the mt6589 datasheet and for me it doesn't look like as
> if this drivers is compatible to mt6589.

Matthias - the compatible is "mt6595", not mt6589 :-).
Which datasheet did you check?

-Dan


>
> DISP_PWM_CON_0 offset 0x10 maps to interrupt enable register and
> DISP_PWM_CON_1 offset 0x14 maps to interrupt status register.
>
> This looks wrong to me, as you use both registers to write clock divider and
> clock period.
>
> Regarding that this is v6 of the patch set, I would propose that you just drop
> the compatible string for mt6589 or you implement the register offset on basis
> of the compatible string so that mt6589 can you the driver as well.
>
> Best regards,
> Matthias
> --
> 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/
Matthias Brugger July 24, 2015, 12:56 p.m. UTC | #3
On Friday, July 24, 2015 05:00:13 PM Daniel Kurtz wrote:
> On Fri, Jul 24, 2015 at 4:40 PM, Matthias Brugger
> 
> <matthias.bgg@gmail.com> wrote:
> > On Monday, July 20, 2015 04:17:15 PM YH Huang wrote:
> >> Document the device-tree binding of MediatTek display PWM.
> >> The PWM has one channel to control the backlight brightness for display.
> >> It supports MT8173 and MT6595.
> >> 
> >> Signed-off-by: YH Huang <yh.huang@mediatek.com>
> >> ---
> >> 
> >>  .../devicetree/bindings/pwm/pwm-mtk-disp.txt       | 42
> >> 
> >> ++++++++++++++++++++++ 1 file changed, 42 insertions(+)
> >> 
> >>  create mode 100644
> >>  Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >> 
> >> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >> b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt new file mode
> >> 100644
> >> index 0000000..f8f59ba
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >> @@ -0,0 +1,42 @@
> >> +MediaTek display PWM controller
> >> +
> >> +Required properties:
> >> + - compatible: should be "mediatek,<name>-disp-pwm":
> >> +   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
> >> +   - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
> > 
> > I had another look on the mt6589 datasheet and for me it doesn't look like
> > as if this drivers is compatible to mt6589.
> 
> Matthias - the compatible is "mt6595", not mt6589 :-).
> Which datasheet did you check?
> 

In further versions the driver stated it support mt6589, so I didn' realize 
that it is mt6595 now. Logically I checked mt6589 datasheet... :(

Sorry for the noise!
Thierry Reding Aug. 17, 2015, 1:23 p.m. UTC | #4
On Mon, Jul 20, 2015 at 04:17:15PM +0800, YH Huang wrote:
> Document the device-tree binding of MediatTek display PWM.

I already mentioned this a while back: s/MediatTek/MediaTek/.

Thierry
YH Huang Aug. 18, 2015, 2:23 a.m. UTC | #5
On Mon, 2015-08-17 at 15:23 +0200, Thierry Reding wrote:
> On Mon, Jul 20, 2015 at 04:17:15PM +0800, YH Huang wrote:
> > Document the device-tree binding of MediatTek display PWM.
> 
> I already mentioned this a while back: s/MediatTek/MediaTek/.
> 
Sorry, I will correct it to "MediaTek".

Regards,
YH Huang

Patch
diff mbox

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
new file mode 100644
index 0000000..f8f59ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
@@ -0,0 +1,42 @@ 
+MediaTek display PWM controller
+
+Required properties:
+ - compatible: should be "mediatek,<name>-disp-pwm":
+   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
+   - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
+ - reg: physical base address and length of the controller's registers.
+ - #pwm-cells: must be 2. See pwm.txt in this directory for a description of
+   the cell format.
+ - clocks: phandle and clock specifier of the PWM reference clock.
+ - clock-names: must contain the following:
+   - "main": clock used to generate PWM signals.
+   - "mm": sync signals from the modules of mmsys.
+ - pinctrl-names: Must contain a "default" entry.
+ - pinctrl-0: One property must exist for each entry in pinctrl-names.
+   See pinctrl/pinctrl-bindings.txt for details of the property values.
+
+Example:
+	pwm0: pwm@1401e000 {
+		compatible = "mediatek,mt8173-disp-pwm",
+			     "mediatek,mt6595-disp-pwm";
+		reg = <0 0x1401e000 0 0x1000>;
+		#pwm-cells = <2>;
+		clocks = <&mmsys CLK_MM_DISP_PWM026M>,
+			 <&mmsys CLK_MM_DISP_PWM0MM>;
+		clock-names = "main", "mm";
+		pinctrl-names = "default";
+		pinctrl-0 = <&disp_pwm0_pins>;
+	};
+
+	backlight_lcd: backlight_lcd {
+		compatible = "pwm-backlight";
+		pwms = <&pwm0 0 1000000>;
+		brightness-levels = <
+			  0  16  32  48  64  80  96 112
+			128 144 160 176 192 208 224 240
+			255
+		>;
+		default-brightness-level = <9>;
+		power-supply = <&mt6397_vio18_reg>;
+		enable-gpios = <&pio 95 GPIO_ACTIVE_HIGH>;
+	};