[PATCH RESEND 0/3] Support Mediatek Soc MT2701 disp pwm

[PATCH RESEND 0/3] Support Mediatek Soc MT2701 disp pwm

[Weiqing Kong]

Sorry for sending many times, because of company
permission settings, many people and groups can't
receive these patches before.

These series patches modify pwm-mtk-disp.c
and dts/dtsi config to support MT2701 disp pwm.

Weiqing Kong (3):
  dt-bindings: pwm: Add MediaTek display PWM bindings
  pwm: Add MediaTek MT2701 display PWM driver support
  dts: Add disp bls dsti and dts config for MT2701

 .../devicetree/bindings/pwm/pwm-mtk-disp.txt       |  5 +-
 arch/arm/boot/dts/mt2701-evb.dts                   | 29 +++++++
 arch/arm/boot/dts/mt2701.dtsi                      |  9 +++
 drivers/pwm/pwm-mtk-disp.c                         | 89 +++++++++++++++++-----
 4 files changed, 113 insertions(+), 19 deletions(-)

-- 
1.8.1.1.dirty

[PATCH RESEND 0/3] Support Mediatek Soc MT2701 disp pwm

[Weiqing Kong]

Sorry for sending many times, because of company
permission settings, many people and groups can't
receive these patches before.

These series patches modify pwm-mtk-disp.c
and dts/dtsi config to support MT2701 disp pwm.

Weiqing Kong (3):
  dt-bindings: pwm: Add MediaTek display PWM bindings
  pwm: Add MediaTek MT2701 display PWM driver support
  dts: Add disp bls dsti and dts config for MT2701

 .../devicetree/bindings/pwm/pwm-mtk-disp.txt       |  5 +-
 arch/arm/boot/dts/mt2701-evb.dts                   | 29 +++++++
 arch/arm/boot/dts/mt2701.dtsi                      |  9 +++
 drivers/pwm/pwm-mtk-disp.c                         | 89 +++++++++++++++++-----
 4 files changed, 113 insertions(+), 19 deletions(-)

-- 
1.8.1.1.dirty

[PATCH RESEND 0/3] Support Mediatek Soc MT2701 disp pwm

[Weiqing Kong]

Sorry for sending many times, because of company
permission settings, many people and groups can't
receive these patches before.

These series patches modify pwm-mtk-disp.c
and dts/dtsi config to support MT2701 disp pwm.

Weiqing Kong (3):
  dt-bindings: pwm: Add MediaTek display PWM bindings
  pwm: Add MediaTek MT2701 display PWM driver support
  dts: Add disp bls dsti and dts config for MT2701

 .../devicetree/bindings/pwm/pwm-mtk-disp.txt       |  5 +-
 arch/arm/boot/dts/mt2701-evb.dts                   | 29 +++++++
 arch/arm/boot/dts/mt2701.dtsi                      |  9 +++
 drivers/pwm/pwm-mtk-disp.c                         | 89 +++++++++++++++++-----
 4 files changed, 113 insertions(+), 19 deletions(-)

-- 
1.8.1.1.dirty

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[Weiqing Kong]

Add MT2701 compatible string.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
index f8f59ba..f2f2fa9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
@@ -1,9 +1,10 @@
 MediaTek display PWM controller
 
 Required properties:
- - compatible: should be "mediatek,<name>-disp-pwm":
-   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
+ - compatible: should be like these:
+   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
    - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
+   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
-- 
1.8.1.1.dirty

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[Weiqing Kong]

Add MT2701 compatible string.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
index f8f59ba..f2f2fa9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
@@ -1,9 +1,10 @@
 MediaTek display PWM controller
 
 Required properties:
- - compatible: should be "mediatek,<name>-disp-pwm":
-   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
+ - compatible: should be like these:
+   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
    - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
+   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
-- 
1.8.1.1.dirty

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[Weiqing Kong]

Add MT2701 compatible string.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
index f8f59ba..f2f2fa9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
@@ -1,9 +1,10 @@
 MediaTek display PWM controller
 
 Required properties:
- - compatible: should be "mediatek,<name>-disp-pwm":
-   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
+ - compatible: should be like these:
+   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
    - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
+   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
-- 
1.8.1.1.dirty

Re: [PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[Rob Herring]

On Mon, May 30, 2016 at 04:41:50PM +0800, Weiqing Kong wrote:
> Add MT2701 compatible string.
> 
> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> ---
>  Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> index f8f59ba..f2f2fa9 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> @@ -1,9 +1,10 @@
>  MediaTek display PWM controller
>  
>  Required properties:
> - - compatible: should be "mediatek,<name>-disp-pwm":
> -   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
> + - compatible: should be like these:
> +   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.

Why bls instead of pwm?

>     - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
> +   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
> -- 
> 1.8.1.1.dirty
> 

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[Rob Herring]

On Mon, May 30, 2016 at 04:41:50PM +0800, Weiqing Kong wrote:
> Add MT2701 compatible string.
> 
> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> ---
>  Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> index f8f59ba..f2f2fa9 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> @@ -1,9 +1,10 @@
>  MediaTek display PWM controller
>  
>  Required properties:
> - - compatible: should be "mediatek,<name>-disp-pwm":
> -   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
> + - compatible: should be like these:
> +   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.

Why bls instead of pwm?

>     - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
> +   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
> -- 
> 1.8.1.1.dirty
> 

Re: [PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[weiqing kong]

On Thu, 2016-06-02 at 17:45 -0500, Rob Herring wrote:
> On Mon, May 30, 2016 at 04:41:50PM +0800, Weiqing Kong wrote:
> > Add MT2701 compatible string.
> > 
> > Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> > index f8f59ba..f2f2fa9 100644
> > --- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> > @@ -1,9 +1,10 @@
> >  MediaTek display PWM controller
> >  
> >  Required properties:
> > - - compatible: should be "mediatek,<name>-disp-pwm":
> > -   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
> > + - compatible: should be like these:
> > +   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
> 
> Why bls instead of pwm?

In mt6595/mt8173, we have a hardware module named pwm.
In mt2701, we have bls hardware module which includes disp pwm function,
so we named it mt2701-disp-bls rather than mt2701-disp-pwm in order to
match with hardware spec.
thanks.

> 
> >     - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
> > +   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
> > -- 
> > 1.8.1.1.dirty
> > 

Re: [PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[weiqing kong]

On Thu, 2016-06-02 at 17:45 -0500, Rob Herring wrote:
> On Mon, May 30, 2016 at 04:41:50PM +0800, Weiqing Kong wrote:
> > Add MT2701 compatible string.
> > 
> > Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> > index f8f59ba..f2f2fa9 100644
> > --- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> > @@ -1,9 +1,10 @@
> >  MediaTek display PWM controller
> >  
> >  Required properties:
> > - - compatible: should be "mediatek,<name>-disp-pwm":
> > -   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
> > + - compatible: should be like these:
> > +   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
> 
> Why bls instead of pwm?

In mt6595/mt8173, we have a hardware module named pwm.
In mt2701, we have bls hardware module which includes disp pwm function,
so we named it mt2701-disp-bls rather than mt2701-disp-pwm in order to
match with hardware spec.
thanks.

> 
> >     - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
> > +   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
> > -- 
> > 1.8.1.1.dirty
> > 

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[weiqing kong]

On Thu, 2016-06-02 at 17:45 -0500, Rob Herring wrote:
> On Mon, May 30, 2016 at 04:41:50PM +0800, Weiqing Kong wrote:
> > Add MT2701 compatible string.
> > 
> > Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> > index f8f59ba..f2f2fa9 100644
> > --- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> > +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> > @@ -1,9 +1,10 @@
> >  MediaTek display PWM controller
> >  
> >  Required properties:
> > - - compatible: should be "mediatek,<name>-disp-pwm":
> > -   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
> > + - compatible: should be like these:
> > +   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
> 
> Why bls instead of pwm?

In mt6595/mt8173, we have a hardware module named pwm.
In mt2701, we have bls hardware module which includes disp pwm function,
so we named it mt2701-disp-bls rather than mt2701-disp-pwm in order to
match with hardware spec.
thanks.

> 
> >     - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
> > +   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
> > -- 
> > 1.8.1.1.dirty
> > 

Re: [PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[Matthias Brugger]

On 03/06/16 08:45, weiqing kong wrote:
> On Thu, 2016-06-02 at 17:45 -0500, Rob Herring wrote:
>> On Mon, May 30, 2016 at 04:41:50PM +0800, Weiqing Kong wrote:
>>> Add MT2701 compatible string.
>>>
>>> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
>>> ---
>>>   Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
>>> index f8f59ba..f2f2fa9 100644
>>> --- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
>>> +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
>>> @@ -1,9 +1,10 @@
>>>   MediaTek display PWM controller
>>>
>>>   Required properties:
>>> - - compatible: should be "mediatek,<name>-disp-pwm":
>>> -   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
>>> + - compatible: should be like these:
>>> +   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
>>
>> Why bls instead of pwm?
>
> In mt6595/mt8173, we have a hardware module named pwm.
> In mt2701, we have bls hardware module which includes disp pwm function,
> so we named it mt2701-disp-bls rather than mt2701-disp-pwm in order to
> match with hardware spec.
> thanks.
>

 From what I understand pwm module and disp pwm in mt6589/mt8173 is 
independent from the disp pwm. Actually the disp pwm in mt6589 is part 
of the bls module, so this should me mediatek,mt2701-disp-pwm as well

Thanks,
Matthias

>>
>>>      - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
>>> +   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
>>> --
>>> 1.8.1.1.dirty
>>>
>
>

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[Matthias Brugger]

On 03/06/16 08:45, weiqing kong wrote:
> On Thu, 2016-06-02 at 17:45 -0500, Rob Herring wrote:
>> On Mon, May 30, 2016 at 04:41:50PM +0800, Weiqing Kong wrote:
>>> Add MT2701 compatible string.
>>>
>>> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
>>> ---
>>>   Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
>>> index f8f59ba..f2f2fa9 100644
>>> --- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
>>> +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
>>> @@ -1,9 +1,10 @@
>>>   MediaTek display PWM controller
>>>
>>>   Required properties:
>>> - - compatible: should be "mediatek,<name>-disp-pwm":
>>> -   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
>>> + - compatible: should be like these:
>>> +   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
>>
>> Why bls instead of pwm?
>
> In mt6595/mt8173, we have a hardware module named pwm.
> In mt2701, we have bls hardware module which includes disp pwm function,
> so we named it mt2701-disp-bls rather than mt2701-disp-pwm in order to
> match with hardware spec.
> thanks.
>

 From what I understand pwm module and disp pwm in mt6589/mt8173 is 
independent from the disp pwm. Actually the disp pwm in mt6589 is part 
of the bls module, so this should me mediatek,mt2701-disp-pwm as well

Thanks,
Matthias

>>
>>>      - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
>>> +   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
>>> --
>>> 1.8.1.1.dirty
>>>
>
>

Re: [PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[weiqing kong]

On Fri, 2016-06-03 at 17:03 +0200, Matthias Brugger wrote:
> 
> On 03/06/16 08:45, weiqing kong wrote:
> > On Thu, 2016-06-02 at 17:45 -0500, Rob Herring wrote:
> >> On Mon, May 30, 2016 at 04:41:50PM +0800, Weiqing Kong wrote:
> >>> Add MT2701 compatible string.
> >>>
> >>> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> >>> ---
> >>>   Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
> >>>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >>> index f8f59ba..f2f2fa9 100644
> >>> --- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >>> +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >>> @@ -1,9 +1,10 @@
> >>>   MediaTek display PWM controller
> >>>
> >>>   Required properties:
> >>> - - compatible: should be "mediatek,<name>-disp-pwm":
> >>> -   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
> >>> + - compatible: should be like these:
> >>> +   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
> >>
> >> Why bls instead of pwm?
> >
> > In mt6595/mt8173, we have a hardware module named pwm.
> > In mt2701, we have bls hardware module which includes disp pwm function,
> > so we named it mt2701-disp-bls rather than mt2701-disp-pwm in order to
> > match with hardware spec.
> > thanks.
> >
> 
>  From what I understand pwm module and disp pwm in mt6589/mt8173 is 
> independent from the disp pwm. Actually the disp pwm in mt6589 is part 
> of the bls module, so this should me mediatek,mt2701-disp-pwm as well
> 
> Thanks,
> Matthias
> 

ok, I will modify it into mediatek,mt2701-disp-pwm. 

> >>
> >>>      - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
> >>> +   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
> >>> --
> >>> 1.8.1.1.dirty
> >>>
> >
> >

Re: [PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[weiqing kong]

On Fri, 2016-06-03 at 17:03 +0200, Matthias Brugger wrote:
> 
> On 03/06/16 08:45, weiqing kong wrote:
> > On Thu, 2016-06-02 at 17:45 -0500, Rob Herring wrote:
> >> On Mon, May 30, 2016 at 04:41:50PM +0800, Weiqing Kong wrote:
> >>> Add MT2701 compatible string.
> >>>
> >>> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> >>> ---
> >>>   Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
> >>>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >>> index f8f59ba..f2f2fa9 100644
> >>> --- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >>> +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >>> @@ -1,9 +1,10 @@
> >>>   MediaTek display PWM controller
> >>>
> >>>   Required properties:
> >>> - - compatible: should be "mediatek,<name>-disp-pwm":
> >>> -   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
> >>> + - compatible: should be like these:
> >>> +   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
> >>
> >> Why bls instead of pwm?
> >
> > In mt6595/mt8173, we have a hardware module named pwm.
> > In mt2701, we have bls hardware module which includes disp pwm function,
> > so we named it mt2701-disp-bls rather than mt2701-disp-pwm in order to
> > match with hardware spec.
> > thanks.
> >
> 
>  From what I understand pwm module and disp pwm in mt6589/mt8173 is 
> independent from the disp pwm. Actually the disp pwm in mt6589 is part 
> of the bls module, so this should me mediatek,mt2701-disp-pwm as well
> 
> Thanks,
> Matthias
> 

ok, I will modify it into mediatek,mt2701-disp-pwm. 

> >>
> >>>      - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
> >>> +   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
> >>> --
> >>> 1.8.1.1.dirty
> >>>
> >
> >

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[weiqing kong]

On Fri, 2016-06-03 at 17:03 +0200, Matthias Brugger wrote:
> 
> On 03/06/16 08:45, weiqing kong wrote:
> > On Thu, 2016-06-02 at 17:45 -0500, Rob Herring wrote:
> >> On Mon, May 30, 2016 at 04:41:50PM +0800, Weiqing Kong wrote:
> >>> Add MT2701 compatible string.
> >>>
> >>> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> >>> ---
> >>>   Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
> >>>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >>> index f8f59ba..f2f2fa9 100644
> >>> --- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >>> +++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
> >>> @@ -1,9 +1,10 @@
> >>>   MediaTek display PWM controller
> >>>
> >>>   Required properties:
> >>> - - compatible: should be "mediatek,<name>-disp-pwm":
> >>> -   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
> >>> + - compatible: should be like these:
> >>> +   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
> >>
> >> Why bls instead of pwm?
> >
> > In mt6595/mt8173, we have a hardware module named pwm.
> > In mt2701, we have bls hardware module which includes disp pwm function,
> > so we named it mt2701-disp-bls rather than mt2701-disp-pwm in order to
> > match with hardware spec.
> > thanks.
> >
> 
>  From what I understand pwm module and disp pwm in mt6589/mt8173 is 
> independent from the disp pwm. Actually the disp pwm in mt6589 is part 
> of the bls module, so this should me mediatek,mt2701-disp-pwm as well
> 
> Thanks,
> Matthias
> 

ok, I will modify it into mediatek,mt2701-disp-pwm. 

> >>
> >>>      - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
> >>> +   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
> >>> --
> >>> 1.8.1.1.dirty
> >>>
> >
> >

[PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[Weiqing Kong]

Use the mtk_pwm_data struction to define different registers
and add MT2701 specific register operations, such as MT2701
doesn't have commit register, needs to disable double buffer
before writing register, and needs to select manual mode
and use PWM_PERIOD/PWM_HIGH_WIDTH.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 72 insertions(+), 17 deletions(-)

diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
index 0ad3385..03b9c9e 100644
--- a/drivers/pwm/pwm-mtk-disp.c
+++ b/drivers/pwm/pwm-mtk-disp.c
@@ -18,33 +18,44 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
 #include <linux/slab.h>
 
 #define DISP_PWM_EN		0x00
-#define PWM_ENABLE_MASK		BIT(0)
 
 #define DISP_PWM_COMMIT		0x08
 #define PWM_COMMIT_MASK		BIT(0)
 
-#define DISP_PWM_CON_0		0x10
 #define PWM_CLKDIV_SHIFT	16
 #define PWM_CLKDIV_MAX		0x3ff
 #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
 
-#define DISP_PWM_CON_1		0x14
 #define PWM_PERIOD_BIT_WIDTH	12
 #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
 
 #define PWM_HIGH_WIDTH_SHIFT	16
 #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
 
+#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
+#define MT2701_PWM_BLS_DEBUG		0xb0
+#define MT2701_PWM_BLS_DEBUG_MASK	0x3
+
+struct mtk_pwm_data {
+	unsigned int enable_bit;
+	unsigned int con0;
+	unsigned int con0_sel;
+	unsigned int con1;
+	bool have_commit_reg;
+};
+
 struct mtk_disp_pwm {
 	struct pwm_chip chip;
 	struct clk *clk_main;
 	struct clk *clk_mm;
 	void __iomem *base;
+	const struct mtk_pwm_data *data;
 };
 
 static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
@@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		return err;
 	}
 
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
+	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
+				 PWM_CLKDIV_MASK,
 				 clk_div << PWM_CLKDIV_SHIFT);
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
-				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
+	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
+				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
+				 value);
+	if (mdp->data->have_commit_reg) {
+		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
+					 PWM_COMMIT_MASK, 0x1);
+		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
+					 PWM_COMMIT_MASK, 0x0);
+	}
 
 	clk_disable(mdp->clk_mm);
 	clk_disable(mdp->clk_main);
@@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 		return err;
 	}
 
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
+	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
+				 mdp->data->enable_bit,
+				 mdp->data->enable_bit);
 
 	return 0;
 }
@@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
 
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
+	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
+				 mdp->data->enable_bit, 0x0);
 
 	clk_disable(mdp->clk_mm);
 	clk_disable(mdp->clk_main);
@@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
 	.owner = THIS_MODULE,
 };
 
+static const struct mtk_pwm_data mt8173_pwm_data = {
+	.enable_bit = BIT(0),
+	.con0 = 0x10,
+	.con0_sel = 0x0,
+	.con1 = 0x14,
+	.have_commit_reg = true,
+};
+
+static const struct mtk_pwm_data mt2701_pwm_data = {
+	.enable_bit = BIT(16),
+	.con0 = 0xa8,
+	.con0_sel = 0x2,
+	.con1 = 0xac,
+	.have_commit_reg = false,
+};
+
+static const struct of_device_id mtk_disp_pwm_of_match[] = {
+	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
+	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
+	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
+
 static int mtk_disp_pwm_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *id;
 	struct mtk_disp_pwm *mdp;
 	struct resource *r;
 	int ret;
 
+	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
+	if (!id)
+		return -EINVAL;
+
 	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
 	if (!mdp)
 		return -ENOMEM;
@@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
 	mdp->chip.ops = &mtk_disp_pwm_ops;
 	mdp->chip.base = -1;
 	mdp->chip.npwm = 1;
+	mdp->data = id->data;
 
 	ret = pwmchip_add(&mdp->chip);
 	if (ret < 0) {
@@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, mdp);
 
+	/*
+	 * For MT2701, disable double buffer before writing register
+	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
+	 */
+	if (!mdp->data->have_commit_reg) {
+		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
+					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
+		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
+					 MT2701_PWM_MANUAL_SEL_MASK,
+					 mdp->data->con0_sel);
+	}
+
 	return 0;
 
 disable_clk_mm:
@@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
 	return ret;
 }
 
-static const struct of_device_id mtk_disp_pwm_of_match[] = {
-	{ .compatible = "mediatek,mt8173-disp-pwm" },
-	{ .compatible = "mediatek,mt6595-disp-pwm" },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
-
 static struct platform_driver mtk_disp_pwm_driver = {
 	.driver = {
 		.name = "mediatek-disp-pwm",
-- 
1.8.1.1.dirty

[PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[Weiqing Kong]

Use the mtk_pwm_data struction to define different registers
and add MT2701 specific register operations, such as MT2701
doesn't have commit register, needs to disable double buffer
before writing register, and needs to select manual mode
and use PWM_PERIOD/PWM_HIGH_WIDTH.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 72 insertions(+), 17 deletions(-)

diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
index 0ad3385..03b9c9e 100644
--- a/drivers/pwm/pwm-mtk-disp.c
+++ b/drivers/pwm/pwm-mtk-disp.c
@@ -18,33 +18,44 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
 #include <linux/slab.h>
 
 #define DISP_PWM_EN		0x00
-#define PWM_ENABLE_MASK		BIT(0)
 
 #define DISP_PWM_COMMIT		0x08
 #define PWM_COMMIT_MASK		BIT(0)
 
-#define DISP_PWM_CON_0		0x10
 #define PWM_CLKDIV_SHIFT	16
 #define PWM_CLKDIV_MAX		0x3ff
 #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
 
-#define DISP_PWM_CON_1		0x14
 #define PWM_PERIOD_BIT_WIDTH	12
 #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
 
 #define PWM_HIGH_WIDTH_SHIFT	16
 #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
 
+#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
+#define MT2701_PWM_BLS_DEBUG		0xb0
+#define MT2701_PWM_BLS_DEBUG_MASK	0x3
+
+struct mtk_pwm_data {
+	unsigned int enable_bit;
+	unsigned int con0;
+	unsigned int con0_sel;
+	unsigned int con1;
+	bool have_commit_reg;
+};
+
 struct mtk_disp_pwm {
 	struct pwm_chip chip;
 	struct clk *clk_main;
 	struct clk *clk_mm;
 	void __iomem *base;
+	const struct mtk_pwm_data *data;
 };
 
 static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
@@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		return err;
 	}
 
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
+	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
+				 PWM_CLKDIV_MASK,
 				 clk_div << PWM_CLKDIV_SHIFT);
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
-				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
+	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
+				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
+				 value);
+	if (mdp->data->have_commit_reg) {
+		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
+					 PWM_COMMIT_MASK, 0x1);
+		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
+					 PWM_COMMIT_MASK, 0x0);
+	}
 
 	clk_disable(mdp->clk_mm);
 	clk_disable(mdp->clk_main);
@@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 		return err;
 	}
 
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
+	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
+				 mdp->data->enable_bit,
+				 mdp->data->enable_bit);
 
 	return 0;
 }
@@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
 
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
+	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
+				 mdp->data->enable_bit, 0x0);
 
 	clk_disable(mdp->clk_mm);
 	clk_disable(mdp->clk_main);
@@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
 	.owner = THIS_MODULE,
 };
 
+static const struct mtk_pwm_data mt8173_pwm_data = {
+	.enable_bit = BIT(0),
+	.con0 = 0x10,
+	.con0_sel = 0x0,
+	.con1 = 0x14,
+	.have_commit_reg = true,
+};
+
+static const struct mtk_pwm_data mt2701_pwm_data = {
+	.enable_bit = BIT(16),
+	.con0 = 0xa8,
+	.con0_sel = 0x2,
+	.con1 = 0xac,
+	.have_commit_reg = false,
+};
+
+static const struct of_device_id mtk_disp_pwm_of_match[] = {
+	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
+	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
+	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
+
 static int mtk_disp_pwm_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *id;
 	struct mtk_disp_pwm *mdp;
 	struct resource *r;
 	int ret;
 
+	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
+	if (!id)
+		return -EINVAL;
+
 	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
 	if (!mdp)
 		return -ENOMEM;
@@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
 	mdp->chip.ops = &mtk_disp_pwm_ops;
 	mdp->chip.base = -1;
 	mdp->chip.npwm = 1;
+	mdp->data = id->data;
 
 	ret = pwmchip_add(&mdp->chip);
 	if (ret < 0) {
@@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, mdp);
 
+	/*
+	 * For MT2701, disable double buffer before writing register
+	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
+	 */
+	if (!mdp->data->have_commit_reg) {
+		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
+					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
+		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
+					 MT2701_PWM_MANUAL_SEL_MASK,
+					 mdp->data->con0_sel);
+	}
+
 	return 0;
 
 disable_clk_mm:
@@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
 	return ret;
 }
 
-static const struct of_device_id mtk_disp_pwm_of_match[] = {
-	{ .compatible = "mediatek,mt8173-disp-pwm" },
-	{ .compatible = "mediatek,mt6595-disp-pwm" },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
-
 static struct platform_driver mtk_disp_pwm_driver = {
 	.driver = {
 		.name = "mediatek-disp-pwm",
-- 
1.8.1.1.dirty

[PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[Weiqing Kong]

Use the mtk_pwm_data struction to define different registers
and add MT2701 specific register operations, such as MT2701
doesn't have commit register, needs to disable double buffer
before writing register, and needs to select manual mode
and use PWM_PERIOD/PWM_HIGH_WIDTH.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 72 insertions(+), 17 deletions(-)

diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
index 0ad3385..03b9c9e 100644
--- a/drivers/pwm/pwm-mtk-disp.c
+++ b/drivers/pwm/pwm-mtk-disp.c
@@ -18,33 +18,44 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
 #include <linux/slab.h>
 
 #define DISP_PWM_EN		0x00
-#define PWM_ENABLE_MASK		BIT(0)
 
 #define DISP_PWM_COMMIT		0x08
 #define PWM_COMMIT_MASK		BIT(0)
 
-#define DISP_PWM_CON_0		0x10
 #define PWM_CLKDIV_SHIFT	16
 #define PWM_CLKDIV_MAX		0x3ff
 #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
 
-#define DISP_PWM_CON_1		0x14
 #define PWM_PERIOD_BIT_WIDTH	12
 #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
 
 #define PWM_HIGH_WIDTH_SHIFT	16
 #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
 
+#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
+#define MT2701_PWM_BLS_DEBUG		0xb0
+#define MT2701_PWM_BLS_DEBUG_MASK	0x3
+
+struct mtk_pwm_data {
+	unsigned int enable_bit;
+	unsigned int con0;
+	unsigned int con0_sel;
+	unsigned int con1;
+	bool have_commit_reg;
+};
+
 struct mtk_disp_pwm {
 	struct pwm_chip chip;
 	struct clk *clk_main;
 	struct clk *clk_mm;
 	void __iomem *base;
+	const struct mtk_pwm_data *data;
 };
 
 static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
@@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		return err;
 	}
 
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
+	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
+				 PWM_CLKDIV_MASK,
 				 clk_div << PWM_CLKDIV_SHIFT);
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
-				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
+	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
+				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
+				 value);
+	if (mdp->data->have_commit_reg) {
+		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
+					 PWM_COMMIT_MASK, 0x1);
+		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
+					 PWM_COMMIT_MASK, 0x0);
+	}
 
 	clk_disable(mdp->clk_mm);
 	clk_disable(mdp->clk_main);
@@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 		return err;
 	}
 
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
+	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
+				 mdp->data->enable_bit,
+				 mdp->data->enable_bit);
 
 	return 0;
 }
@@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
 
-	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
+	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
+				 mdp->data->enable_bit, 0x0);
 
 	clk_disable(mdp->clk_mm);
 	clk_disable(mdp->clk_main);
@@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
 	.owner = THIS_MODULE,
 };
 
+static const struct mtk_pwm_data mt8173_pwm_data = {
+	.enable_bit = BIT(0),
+	.con0 = 0x10,
+	.con0_sel = 0x0,
+	.con1 = 0x14,
+	.have_commit_reg = true,
+};
+
+static const struct mtk_pwm_data mt2701_pwm_data = {
+	.enable_bit = BIT(16),
+	.con0 = 0xa8,
+	.con0_sel = 0x2,
+	.con1 = 0xac,
+	.have_commit_reg = false,
+};
+
+static const struct of_device_id mtk_disp_pwm_of_match[] = {
+	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
+	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
+	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
+
 static int mtk_disp_pwm_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *id;
 	struct mtk_disp_pwm *mdp;
 	struct resource *r;
 	int ret;
 
+	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
+	if (!id)
+		return -EINVAL;
+
 	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
 	if (!mdp)
 		return -ENOMEM;
@@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
 	mdp->chip.ops = &mtk_disp_pwm_ops;
 	mdp->chip.base = -1;
 	mdp->chip.npwm = 1;
+	mdp->data = id->data;
 
 	ret = pwmchip_add(&mdp->chip);
 	if (ret < 0) {
@@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, mdp);
 
+	/*
+	 * For MT2701, disable double buffer before writing register
+	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
+	 */
+	if (!mdp->data->have_commit_reg) {
+		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
+					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
+		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
+					 MT2701_PWM_MANUAL_SEL_MASK,
+					 mdp->data->con0_sel);
+	}
+
 	return 0;
 
 disable_clk_mm:
@@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
 	return ret;
 }
 
-static const struct of_device_id mtk_disp_pwm_of_match[] = {
-	{ .compatible = "mediatek,mt8173-disp-pwm" },
-	{ .compatible = "mediatek,mt6595-disp-pwm" },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
-
 static struct platform_driver mtk_disp_pwm_driver = {
 	.driver = {
 		.name = "mediatek-disp-pwm",
-- 
1.8.1.1.dirty

Re: [PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[Matthias Brugger]

On 30/05/16 10:41, Weiqing Kong wrote:
> Use the mtk_pwm_data struction to define different registers
> and add MT2701 specific register operations, such as MT2701
> doesn't have commit register, needs to disable double buffer
> before writing register, and needs to select manual mode
> and use PWM_PERIOD/PWM_HIGH_WIDTH.
>
> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> ---
>   drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 72 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> index 0ad3385..03b9c9e 100644
> --- a/drivers/pwm/pwm-mtk-disp.c
> +++ b/drivers/pwm/pwm-mtk-disp.c
> @@ -18,33 +18,44 @@
>   #include <linux/io.h>
>   #include <linux/module.h>
>   #include <linux/of.h>
> +#include <linux/of_device.h>
>   #include <linux/platform_device.h>
>   #include <linux/pwm.h>
>   #include <linux/slab.h>
>
>   #define DISP_PWM_EN		0x00
> -#define PWM_ENABLE_MASK		BIT(0)
>
>   #define DISP_PWM_COMMIT		0x08
>   #define PWM_COMMIT_MASK		BIT(0)
>
> -#define DISP_PWM_CON_0		0x10
>   #define PWM_CLKDIV_SHIFT	16
>   #define PWM_CLKDIV_MAX		0x3ff
>   #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
>
> -#define DISP_PWM_CON_1		0x14
>   #define PWM_PERIOD_BIT_WIDTH	12
>   #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
>
>   #define PWM_HIGH_WIDTH_SHIFT	16
>   #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
>
> +#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
> +#define MT2701_PWM_BLS_DEBUG		0xb0

Do we need to set magic 0x3 in the debug register? If so, this should be 
part of mtk_pwm_data, just as con0_sel.

> +#define MT2701_PWM_BLS_DEBUG_MASK	0x3
> +
> +struct mtk_pwm_data {
> +	unsigned int enable_bit;
> +	unsigned int con0;
> +	unsigned int con0_sel;
> +	unsigned int con1;
> +	bool have_commit_reg;
> +};
> +
>   struct mtk_disp_pwm {
>   	struct pwm_chip chip;
>   	struct clk *clk_main;
>   	struct clk *clk_mm;
>   	void __iomem *base;
> +	const struct mtk_pwm_data *data;

Couldn't that just be the offset for the commit register and set it to 
0x0 if the commit register is not present. Right now, we suppose the 
DISP_PWM_EN is at offset 0x0, so this should not be a problem.

>   };
>
>   static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
> @@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>   		return err;
>   	}
>
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> +				 PWM_CLKDIV_MASK,
>   				 clk_div << PWM_CLKDIV_SHIFT);
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
> -				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
> +				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
> +				 value);
> +	if (mdp->data->have_commit_reg) {
> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> +					 PWM_COMMIT_MASK, 0x1);
> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> +					 PWM_COMMIT_MASK, 0x0);

should be renamed to MT8173_PWM_COMMIT_MASK, although I'm not really a 
friend of mixing up SoC specific defines with a Soc specific 
mtk_disp_pwm structure.

Regards,
Matthias

> +	}
>
>   	clk_disable(mdp->clk_mm);
>   	clk_disable(mdp->clk_main);
> @@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>   		return err;
>   	}
>
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> +				 mdp->data->enable_bit,
> +				 mdp->data->enable_bit);
>
>   	return 0;
>   }
> @@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>   {
>   	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
>
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> +				 mdp->data->enable_bit, 0x0);
>
>   	clk_disable(mdp->clk_mm);
>   	clk_disable(mdp->clk_main);
> @@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
>   	.owner = THIS_MODULE,
>   };
>
> +static const struct mtk_pwm_data mt8173_pwm_data = {
> +	.enable_bit = BIT(0),
> +	.con0 = 0x10,
> +	.con0_sel = 0x0,
> +	.con1 = 0x14,
> +	.have_commit_reg = true,
> +};
> +
> +static const struct mtk_pwm_data mt2701_pwm_data = {
> +	.enable_bit = BIT(16),
> +	.con0 = 0xa8,
> +	.con0_sel = 0x2,
> +	.con1 = 0xac,
> +	.have_commit_reg = false,
> +};
> +
> +static const struct of_device_id mtk_disp_pwm_of_match[] = {
> +	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
> +	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
> +	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> +
>   static int mtk_disp_pwm_probe(struct platform_device *pdev)
>   {
> +	const struct of_device_id *id;
>   	struct mtk_disp_pwm *mdp;
>   	struct resource *r;
>   	int ret;
>
> +	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
> +	if (!id)
> +		return -EINVAL;
> +
>   	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
>   	if (!mdp)
>   		return -ENOMEM;
> @@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
>   	mdp->chip.ops = &mtk_disp_pwm_ops;
>   	mdp->chip.base = -1;
>   	mdp->chip.npwm = 1;
> +	mdp->data = id->data;
>
>   	ret = pwmchip_add(&mdp->chip);
>   	if (ret < 0) {
> @@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
>
>   	platform_set_drvdata(pdev, mdp);
>
> +	/*
> +	 * For MT2701, disable double buffer before writing register
> +	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
> +	 */
> +	if (!mdp->data->have_commit_reg) {
> +		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
> +					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
> +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> +					 MT2701_PWM_MANUAL_SEL_MASK,
> +					 mdp->data->con0_sel);
> +	}
> +
>   	return 0;
>
>   disable_clk_mm:
> @@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
>   	return ret;
>   }
>
> -static const struct of_device_id mtk_disp_pwm_of_match[] = {
> -	{ .compatible = "mediatek,mt8173-disp-pwm" },
> -	{ .compatible = "mediatek,mt6595-disp-pwm" },
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> -
>   static struct platform_driver mtk_disp_pwm_driver = {
>   	.driver = {
>   		.name = "mediatek-disp-pwm",
>

[PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[Matthias Brugger]

On 30/05/16 10:41, Weiqing Kong wrote:
> Use the mtk_pwm_data struction to define different registers
> and add MT2701 specific register operations, such as MT2701
> doesn't have commit register, needs to disable double buffer
> before writing register, and needs to select manual mode
> and use PWM_PERIOD/PWM_HIGH_WIDTH.
>
> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> ---
>   drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 72 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> index 0ad3385..03b9c9e 100644
> --- a/drivers/pwm/pwm-mtk-disp.c
> +++ b/drivers/pwm/pwm-mtk-disp.c
> @@ -18,33 +18,44 @@
>   #include <linux/io.h>
>   #include <linux/module.h>
>   #include <linux/of.h>
> +#include <linux/of_device.h>
>   #include <linux/platform_device.h>
>   #include <linux/pwm.h>
>   #include <linux/slab.h>
>
>   #define DISP_PWM_EN		0x00
> -#define PWM_ENABLE_MASK		BIT(0)
>
>   #define DISP_PWM_COMMIT		0x08
>   #define PWM_COMMIT_MASK		BIT(0)
>
> -#define DISP_PWM_CON_0		0x10
>   #define PWM_CLKDIV_SHIFT	16
>   #define PWM_CLKDIV_MAX		0x3ff
>   #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
>
> -#define DISP_PWM_CON_1		0x14
>   #define PWM_PERIOD_BIT_WIDTH	12
>   #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
>
>   #define PWM_HIGH_WIDTH_SHIFT	16
>   #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
>
> +#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
> +#define MT2701_PWM_BLS_DEBUG		0xb0

Do we need to set magic 0x3 in the debug register? If so, this should be 
part of mtk_pwm_data, just as con0_sel.

> +#define MT2701_PWM_BLS_DEBUG_MASK	0x3
> +
> +struct mtk_pwm_data {
> +	unsigned int enable_bit;
> +	unsigned int con0;
> +	unsigned int con0_sel;
> +	unsigned int con1;
> +	bool have_commit_reg;
> +};
> +
>   struct mtk_disp_pwm {
>   	struct pwm_chip chip;
>   	struct clk *clk_main;
>   	struct clk *clk_mm;
>   	void __iomem *base;
> +	const struct mtk_pwm_data *data;

Couldn't that just be the offset for the commit register and set it to 
0x0 if the commit register is not present. Right now, we suppose the 
DISP_PWM_EN is at offset 0x0, so this should not be a problem.

>   };
>
>   static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
> @@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>   		return err;
>   	}
>
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> +				 PWM_CLKDIV_MASK,
>   				 clk_div << PWM_CLKDIV_SHIFT);
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
> -				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
> +				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
> +				 value);
> +	if (mdp->data->have_commit_reg) {
> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> +					 PWM_COMMIT_MASK, 0x1);
> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> +					 PWM_COMMIT_MASK, 0x0);

should be renamed to MT8173_PWM_COMMIT_MASK, although I'm not really a 
friend of mixing up SoC specific defines with a Soc specific 
mtk_disp_pwm structure.

Regards,
Matthias

> +	}
>
>   	clk_disable(mdp->clk_mm);
>   	clk_disable(mdp->clk_main);
> @@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>   		return err;
>   	}
>
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> +				 mdp->data->enable_bit,
> +				 mdp->data->enable_bit);
>
>   	return 0;
>   }
> @@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>   {
>   	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
>
> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> +				 mdp->data->enable_bit, 0x0);
>
>   	clk_disable(mdp->clk_mm);
>   	clk_disable(mdp->clk_main);
> @@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
>   	.owner = THIS_MODULE,
>   };
>
> +static const struct mtk_pwm_data mt8173_pwm_data = {
> +	.enable_bit = BIT(0),
> +	.con0 = 0x10,
> +	.con0_sel = 0x0,
> +	.con1 = 0x14,
> +	.have_commit_reg = true,
> +};
> +
> +static const struct mtk_pwm_data mt2701_pwm_data = {
> +	.enable_bit = BIT(16),
> +	.con0 = 0xa8,
> +	.con0_sel = 0x2,
> +	.con1 = 0xac,
> +	.have_commit_reg = false,
> +};
> +
> +static const struct of_device_id mtk_disp_pwm_of_match[] = {
> +	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
> +	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
> +	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> +
>   static int mtk_disp_pwm_probe(struct platform_device *pdev)
>   {
> +	const struct of_device_id *id;
>   	struct mtk_disp_pwm *mdp;
>   	struct resource *r;
>   	int ret;
>
> +	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
> +	if (!id)
> +		return -EINVAL;
> +
>   	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
>   	if (!mdp)
>   		return -ENOMEM;
> @@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
>   	mdp->chip.ops = &mtk_disp_pwm_ops;
>   	mdp->chip.base = -1;
>   	mdp->chip.npwm = 1;
> +	mdp->data = id->data;
>
>   	ret = pwmchip_add(&mdp->chip);
>   	if (ret < 0) {
> @@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
>
>   	platform_set_drvdata(pdev, mdp);
>
> +	/*
> +	 * For MT2701, disable double buffer before writing register
> +	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
> +	 */
> +	if (!mdp->data->have_commit_reg) {
> +		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
> +					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
> +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> +					 MT2701_PWM_MANUAL_SEL_MASK,
> +					 mdp->data->con0_sel);
> +	}
> +
>   	return 0;
>
>   disable_clk_mm:
> @@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
>   	return ret;
>   }
>
> -static const struct of_device_id mtk_disp_pwm_of_match[] = {
> -	{ .compatible = "mediatek,mt8173-disp-pwm" },
> -	{ .compatible = "mediatek,mt6595-disp-pwm" },
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> -
>   static struct platform_driver mtk_disp_pwm_driver = {
>   	.driver = {
>   		.name = "mediatek-disp-pwm",
>

Re: [PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[weiqing kong]

On Fri, 2016-06-03 at 17:35 +0200, Matthias Brugger wrote:
> 
> On 30/05/16 10:41, Weiqing Kong wrote:
> > Use the mtk_pwm_data struction to define different registers
> > and add MT2701 specific register operations, such as MT2701
> > doesn't have commit register, needs to disable double buffer
> > before writing register, and needs to select manual mode
> > and use PWM_PERIOD/PWM_HIGH_WIDTH.
> >
> > Signed-off-by: Weiqing Kong <weiqing.kong-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >   drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
> >   1 file changed, 72 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> > index 0ad3385..03b9c9e 100644
> > --- a/drivers/pwm/pwm-mtk-disp.c
> > +++ b/drivers/pwm/pwm-mtk-disp.c
> > @@ -18,33 +18,44 @@
> >   #include <linux/io.h>
> >   #include <linux/module.h>
> >   #include <linux/of.h>
> > +#include <linux/of_device.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/pwm.h>
> >   #include <linux/slab.h>
> >
> >   #define DISP_PWM_EN		0x00
> > -#define PWM_ENABLE_MASK		BIT(0)
> >
> >   #define DISP_PWM_COMMIT		0x08
> >   #define PWM_COMMIT_MASK		BIT(0)
> >
> > -#define DISP_PWM_CON_0		0x10
> >   #define PWM_CLKDIV_SHIFT	16
> >   #define PWM_CLKDIV_MAX		0x3ff
> >   #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
> >
> > -#define DISP_PWM_CON_1		0x14
> >   #define PWM_PERIOD_BIT_WIDTH	12
> >   #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
> >
> >   #define PWM_HIGH_WIDTH_SHIFT	16
> >   #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
> >
> > +#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
> > +#define MT2701_PWM_BLS_DEBUG		0xb0
> 
> Do we need to set magic 0x3 in the debug register? If so, this should be 
> part of mtk_pwm_data, just as con0_sel.

Do you mean that add bls_debug into struct mtk_pwm_data,
and set 0x3 in mt2701 data, 0x0 in mt8173 data?

> 
> > +#define MT2701_PWM_BLS_DEBUG_MASK	0x3
> > +
> > +struct mtk_pwm_data {
> > +	unsigned int enable_bit;
> > +	unsigned int con0;
> > +	unsigned int con0_sel;
> > +	unsigned int con1;
> > +	bool have_commit_reg;
> > +};
> > +
> >   struct mtk_disp_pwm {
> >   	struct pwm_chip chip;
> >   	struct clk *clk_main;
> >   	struct clk *clk_mm;
> >   	void __iomem *base;
> > +	const struct mtk_pwm_data *data;
> 
> Couldn't that just be the offset for the commit register and set it to 
> 0x0 if the commit register is not present. Right now, we suppose the 
> DISP_PWM_EN is at offset 0x0, so this should not be a problem.
> 

Do you mean that replace have_commit_reg with commit_reg_offset?
just like this?
bool have_commit_reg;
->
unsigned int commit_reg_offset;
and set commit_reg_offset 0x0 in mt2701, 0x1 in mt8173?

> >   };
> >
> >   static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
> > @@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >   		return err;
> >   	}
> >
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
> > +	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> > +				 PWM_CLKDIV_MASK,
> >   				 clk_div << PWM_CLKDIV_SHIFT);
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
> > -				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
> > +	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
> > +				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
> > +				 value);
> > +	if (mdp->data->have_commit_reg) {
> > +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> > +					 PWM_COMMIT_MASK, 0x1);
> > +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> > +					 PWM_COMMIT_MASK, 0x0);
> 
> should be renamed to MT8173_PWM_COMMIT_MASK, although I'm not really a 
> friend of mixing up SoC specific defines with a Soc specific 
> mtk_disp_pwm structure.
> 
> Regards,
> Matthias

ok, I will modify it as you said. thanks for you suggestion.

> 
> > +	}
> >
> >   	clk_disable(mdp->clk_mm);
> >   	clk_disable(mdp->clk_main);
> > @@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> >   		return err;
> >   	}
> >
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
> > +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> > +				 mdp->data->enable_bit,
> > +				 mdp->data->enable_bit);
> >
> >   	return 0;
> >   }
> > @@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >   {
> >   	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
> >
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
> > +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> > +				 mdp->data->enable_bit, 0x0);
> >
> >   	clk_disable(mdp->clk_mm);
> >   	clk_disable(mdp->clk_main);
> > @@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
> >   	.owner = THIS_MODULE,
> >   };
> >
> > +static const struct mtk_pwm_data mt8173_pwm_data = {
> > +	.enable_bit = BIT(0),
> > +	.con0 = 0x10,
> > +	.con0_sel = 0x0,
> > +	.con1 = 0x14,
> > +	.have_commit_reg = true,
> > +};
> > +
> > +static const struct mtk_pwm_data mt2701_pwm_data = {
> > +	.enable_bit = BIT(16),
> > +	.con0 = 0xa8,
> > +	.con0_sel = 0x2,
> > +	.con1 = 0xac,
> > +	.have_commit_reg = false,
> > +};
> > +
> > +static const struct of_device_id mtk_disp_pwm_of_match[] = {
> > +	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
> > +	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
> > +	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> > +
> >   static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >   {
> > +	const struct of_device_id *id;
> >   	struct mtk_disp_pwm *mdp;
> >   	struct resource *r;
> >   	int ret;
> >
> > +	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
> > +	if (!id)
> > +		return -EINVAL;
> > +
> >   	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
> >   	if (!mdp)
> >   		return -ENOMEM;
> > @@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >   	mdp->chip.ops = &mtk_disp_pwm_ops;
> >   	mdp->chip.base = -1;
> >   	mdp->chip.npwm = 1;
> > +	mdp->data = id->data;
> >
> >   	ret = pwmchip_add(&mdp->chip);
> >   	if (ret < 0) {
> > @@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >
> >   	platform_set_drvdata(pdev, mdp);
> >
> > +	/*
> > +	 * For MT2701, disable double buffer before writing register
> > +	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
> > +	 */
> > +	if (!mdp->data->have_commit_reg) {
> > +		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
> > +					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
> > +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> > +					 MT2701_PWM_MANUAL_SEL_MASK,
> > +					 mdp->data->con0_sel);
> > +	}
> > +
> >   	return 0;
> >
> >   disable_clk_mm:
> > @@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
> >   	return ret;
> >   }
> >
> > -static const struct of_device_id mtk_disp_pwm_of_match[] = {
> > -	{ .compatible = "mediatek,mt8173-disp-pwm" },
> > -	{ .compatible = "mediatek,mt6595-disp-pwm" },
> > -	{ }
> > -};
> > -MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> > -
> >   static struct platform_driver mtk_disp_pwm_driver = {
> >   	.driver = {
> >   		.name = "mediatek-disp-pwm",
> >


--
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

Re: [PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[weiqing kong]

On Fri, 2016-06-03 at 17:35 +0200, Matthias Brugger wrote:
> 
> On 30/05/16 10:41, Weiqing Kong wrote:
> > Use the mtk_pwm_data struction to define different registers
> > and add MT2701 specific register operations, such as MT2701
> > doesn't have commit register, needs to disable double buffer
> > before writing register, and needs to select manual mode
> > and use PWM_PERIOD/PWM_HIGH_WIDTH.
> >
> > Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> > ---
> >   drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
> >   1 file changed, 72 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> > index 0ad3385..03b9c9e 100644
> > --- a/drivers/pwm/pwm-mtk-disp.c
> > +++ b/drivers/pwm/pwm-mtk-disp.c
> > @@ -18,33 +18,44 @@
> >   #include <linux/io.h>
> >   #include <linux/module.h>
> >   #include <linux/of.h>
> > +#include <linux/of_device.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/pwm.h>
> >   #include <linux/slab.h>
> >
> >   #define DISP_PWM_EN		0x00
> > -#define PWM_ENABLE_MASK		BIT(0)
> >
> >   #define DISP_PWM_COMMIT		0x08
> >   #define PWM_COMMIT_MASK		BIT(0)
> >
> > -#define DISP_PWM_CON_0		0x10
> >   #define PWM_CLKDIV_SHIFT	16
> >   #define PWM_CLKDIV_MAX		0x3ff
> >   #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
> >
> > -#define DISP_PWM_CON_1		0x14
> >   #define PWM_PERIOD_BIT_WIDTH	12
> >   #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
> >
> >   #define PWM_HIGH_WIDTH_SHIFT	16
> >   #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
> >
> > +#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
> > +#define MT2701_PWM_BLS_DEBUG		0xb0
> 
> Do we need to set magic 0x3 in the debug register? If so, this should be 
> part of mtk_pwm_data, just as con0_sel.

Do you mean that add bls_debug into struct mtk_pwm_data,
and set 0x3 in mt2701 data, 0x0 in mt8173 data?

> 
> > +#define MT2701_PWM_BLS_DEBUG_MASK	0x3
> > +
> > +struct mtk_pwm_data {
> > +	unsigned int enable_bit;
> > +	unsigned int con0;
> > +	unsigned int con0_sel;
> > +	unsigned int con1;
> > +	bool have_commit_reg;
> > +};
> > +
> >   struct mtk_disp_pwm {
> >   	struct pwm_chip chip;
> >   	struct clk *clk_main;
> >   	struct clk *clk_mm;
> >   	void __iomem *base;
> > +	const struct mtk_pwm_data *data;
> 
> Couldn't that just be the offset for the commit register and set it to 
> 0x0 if the commit register is not present. Right now, we suppose the 
> DISP_PWM_EN is at offset 0x0, so this should not be a problem.
> 

Do you mean that replace have_commit_reg with commit_reg_offset?
just like this?
bool have_commit_reg;
->
unsigned int commit_reg_offset;
and set commit_reg_offset 0x0 in mt2701, 0x1 in mt8173?

> >   };
> >
> >   static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
> > @@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >   		return err;
> >   	}
> >
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
> > +	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> > +				 PWM_CLKDIV_MASK,
> >   				 clk_div << PWM_CLKDIV_SHIFT);
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
> > -				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
> > +	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
> > +				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
> > +				 value);
> > +	if (mdp->data->have_commit_reg) {
> > +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> > +					 PWM_COMMIT_MASK, 0x1);
> > +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> > +					 PWM_COMMIT_MASK, 0x0);
> 
> should be renamed to MT8173_PWM_COMMIT_MASK, although I'm not really a 
> friend of mixing up SoC specific defines with a Soc specific 
> mtk_disp_pwm structure.
> 
> Regards,
> Matthias

ok, I will modify it as you said. thanks for you suggestion.

> 
> > +	}
> >
> >   	clk_disable(mdp->clk_mm);
> >   	clk_disable(mdp->clk_main);
> > @@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> >   		return err;
> >   	}
> >
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
> > +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> > +				 mdp->data->enable_bit,
> > +				 mdp->data->enable_bit);
> >
> >   	return 0;
> >   }
> > @@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >   {
> >   	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
> >
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
> > +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> > +				 mdp->data->enable_bit, 0x0);
> >
> >   	clk_disable(mdp->clk_mm);
> >   	clk_disable(mdp->clk_main);
> > @@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
> >   	.owner = THIS_MODULE,
> >   };
> >
> > +static const struct mtk_pwm_data mt8173_pwm_data = {
> > +	.enable_bit = BIT(0),
> > +	.con0 = 0x10,
> > +	.con0_sel = 0x0,
> > +	.con1 = 0x14,
> > +	.have_commit_reg = true,
> > +};
> > +
> > +static const struct mtk_pwm_data mt2701_pwm_data = {
> > +	.enable_bit = BIT(16),
> > +	.con0 = 0xa8,
> > +	.con0_sel = 0x2,
> > +	.con1 = 0xac,
> > +	.have_commit_reg = false,
> > +};
> > +
> > +static const struct of_device_id mtk_disp_pwm_of_match[] = {
> > +	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
> > +	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
> > +	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> > +
> >   static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >   {
> > +	const struct of_device_id *id;
> >   	struct mtk_disp_pwm *mdp;
> >   	struct resource *r;
> >   	int ret;
> >
> > +	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
> > +	if (!id)
> > +		return -EINVAL;
> > +
> >   	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
> >   	if (!mdp)
> >   		return -ENOMEM;
> > @@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >   	mdp->chip.ops = &mtk_disp_pwm_ops;
> >   	mdp->chip.base = -1;
> >   	mdp->chip.npwm = 1;
> > +	mdp->data = id->data;
> >
> >   	ret = pwmchip_add(&mdp->chip);
> >   	if (ret < 0) {
> > @@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >
> >   	platform_set_drvdata(pdev, mdp);
> >
> > +	/*
> > +	 * For MT2701, disable double buffer before writing register
> > +	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
> > +	 */
> > +	if (!mdp->data->have_commit_reg) {
> > +		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
> > +					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
> > +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> > +					 MT2701_PWM_MANUAL_SEL_MASK,
> > +					 mdp->data->con0_sel);
> > +	}
> > +
> >   	return 0;
> >
> >   disable_clk_mm:
> > @@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
> >   	return ret;
> >   }
> >
> > -static const struct of_device_id mtk_disp_pwm_of_match[] = {
> > -	{ .compatible = "mediatek,mt8173-disp-pwm" },
> > -	{ .compatible = "mediatek,mt6595-disp-pwm" },
> > -	{ }
> > -};
> > -MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> > -
> >   static struct platform_driver mtk_disp_pwm_driver = {
> >   	.driver = {
> >   		.name = "mediatek-disp-pwm",
> >

[PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[weiqing kong]

On Fri, 2016-06-03 at 17:35 +0200, Matthias Brugger wrote:
> 
> On 30/05/16 10:41, Weiqing Kong wrote:
> > Use the mtk_pwm_data struction to define different registers
> > and add MT2701 specific register operations, such as MT2701
> > doesn't have commit register, needs to disable double buffer
> > before writing register, and needs to select manual mode
> > and use PWM_PERIOD/PWM_HIGH_WIDTH.
> >
> > Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> > ---
> >   drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
> >   1 file changed, 72 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> > index 0ad3385..03b9c9e 100644
> > --- a/drivers/pwm/pwm-mtk-disp.c
> > +++ b/drivers/pwm/pwm-mtk-disp.c
> > @@ -18,33 +18,44 @@
> >   #include <linux/io.h>
> >   #include <linux/module.h>
> >   #include <linux/of.h>
> > +#include <linux/of_device.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/pwm.h>
> >   #include <linux/slab.h>
> >
> >   #define DISP_PWM_EN		0x00
> > -#define PWM_ENABLE_MASK		BIT(0)
> >
> >   #define DISP_PWM_COMMIT		0x08
> >   #define PWM_COMMIT_MASK		BIT(0)
> >
> > -#define DISP_PWM_CON_0		0x10
> >   #define PWM_CLKDIV_SHIFT	16
> >   #define PWM_CLKDIV_MAX		0x3ff
> >   #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
> >
> > -#define DISP_PWM_CON_1		0x14
> >   #define PWM_PERIOD_BIT_WIDTH	12
> >   #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
> >
> >   #define PWM_HIGH_WIDTH_SHIFT	16
> >   #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
> >
> > +#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
> > +#define MT2701_PWM_BLS_DEBUG		0xb0
> 
> Do we need to set magic 0x3 in the debug register? If so, this should be 
> part of mtk_pwm_data, just as con0_sel.

Do you mean that add bls_debug into struct mtk_pwm_data,
and set 0x3 in mt2701 data, 0x0 in mt8173 data?

> 
> > +#define MT2701_PWM_BLS_DEBUG_MASK	0x3
> > +
> > +struct mtk_pwm_data {
> > +	unsigned int enable_bit;
> > +	unsigned int con0;
> > +	unsigned int con0_sel;
> > +	unsigned int con1;
> > +	bool have_commit_reg;
> > +};
> > +
> >   struct mtk_disp_pwm {
> >   	struct pwm_chip chip;
> >   	struct clk *clk_main;
> >   	struct clk *clk_mm;
> >   	void __iomem *base;
> > +	const struct mtk_pwm_data *data;
> 
> Couldn't that just be the offset for the commit register and set it to 
> 0x0 if the commit register is not present. Right now, we suppose the 
> DISP_PWM_EN is at offset 0x0, so this should not be a problem.
> 

Do you mean that replace have_commit_reg with commit_reg_offset?
just like this?
bool have_commit_reg;
->
unsigned int commit_reg_offset;
and set commit_reg_offset 0x0 in mt2701, 0x1 in mt8173?

> >   };
> >
> >   static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
> > @@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >   		return err;
> >   	}
> >
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
> > +	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> > +				 PWM_CLKDIV_MASK,
> >   				 clk_div << PWM_CLKDIV_SHIFT);
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
> > -				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
> > +	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
> > +				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
> > +				 value);
> > +	if (mdp->data->have_commit_reg) {
> > +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> > +					 PWM_COMMIT_MASK, 0x1);
> > +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> > +					 PWM_COMMIT_MASK, 0x0);
> 
> should be renamed to MT8173_PWM_COMMIT_MASK, although I'm not really a 
> friend of mixing up SoC specific defines with a Soc specific 
> mtk_disp_pwm structure.
> 
> Regards,
> Matthias

ok, I will modify it as you said. thanks for you suggestion.

> 
> > +	}
> >
> >   	clk_disable(mdp->clk_mm);
> >   	clk_disable(mdp->clk_main);
> > @@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> >   		return err;
> >   	}
> >
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
> > +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> > +				 mdp->data->enable_bit,
> > +				 mdp->data->enable_bit);
> >
> >   	return 0;
> >   }
> > @@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >   {
> >   	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
> >
> > -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
> > +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> > +				 mdp->data->enable_bit, 0x0);
> >
> >   	clk_disable(mdp->clk_mm);
> >   	clk_disable(mdp->clk_main);
> > @@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
> >   	.owner = THIS_MODULE,
> >   };
> >
> > +static const struct mtk_pwm_data mt8173_pwm_data = {
> > +	.enable_bit = BIT(0),
> > +	.con0 = 0x10,
> > +	.con0_sel = 0x0,
> > +	.con1 = 0x14,
> > +	.have_commit_reg = true,
> > +};
> > +
> > +static const struct mtk_pwm_data mt2701_pwm_data = {
> > +	.enable_bit = BIT(16),
> > +	.con0 = 0xa8,
> > +	.con0_sel = 0x2,
> > +	.con1 = 0xac,
> > +	.have_commit_reg = false,
> > +};
> > +
> > +static const struct of_device_id mtk_disp_pwm_of_match[] = {
> > +	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
> > +	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
> > +	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> > +
> >   static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >   {
> > +	const struct of_device_id *id;
> >   	struct mtk_disp_pwm *mdp;
> >   	struct resource *r;
> >   	int ret;
> >
> > +	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
> > +	if (!id)
> > +		return -EINVAL;
> > +
> >   	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
> >   	if (!mdp)
> >   		return -ENOMEM;
> > @@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >   	mdp->chip.ops = &mtk_disp_pwm_ops;
> >   	mdp->chip.base = -1;
> >   	mdp->chip.npwm = 1;
> > +	mdp->data = id->data;
> >
> >   	ret = pwmchip_add(&mdp->chip);
> >   	if (ret < 0) {
> > @@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >
> >   	platform_set_drvdata(pdev, mdp);
> >
> > +	/*
> > +	 * For MT2701, disable double buffer before writing register
> > +	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
> > +	 */
> > +	if (!mdp->data->have_commit_reg) {
> > +		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
> > +					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
> > +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> > +					 MT2701_PWM_MANUAL_SEL_MASK,
> > +					 mdp->data->con0_sel);
> > +	}
> > +
> >   	return 0;
> >
> >   disable_clk_mm:
> > @@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
> >   	return ret;
> >   }
> >
> > -static const struct of_device_id mtk_disp_pwm_of_match[] = {
> > -	{ .compatible = "mediatek,mt8173-disp-pwm" },
> > -	{ .compatible = "mediatek,mt6595-disp-pwm" },
> > -	{ }
> > -};
> > -MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> > -
> >   static struct platform_driver mtk_disp_pwm_driver = {
> >   	.driver = {
> >   		.name = "mediatek-disp-pwm",
> >

Re: [PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[Matthias Brugger]

On 08/06/16 09:21, weiqing kong wrote:
> On Fri, 2016-06-03 at 17:35 +0200, Matthias Brugger wrote:
>>
>> On 30/05/16 10:41, Weiqing Kong wrote:
>>> Use the mtk_pwm_data struction to define different registers
>>> and add MT2701 specific register operations, such as MT2701
>>> doesn't have commit register, needs to disable double buffer
>>> before writing register, and needs to select manual mode
>>> and use PWM_PERIOD/PWM_HIGH_WIDTH.
>>>
>>> Signed-off-by: Weiqing Kong <weiqing.kong-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>> ---
>>>    drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
>>>    1 file changed, 72 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
>>> index 0ad3385..03b9c9e 100644
>>> --- a/drivers/pwm/pwm-mtk-disp.c
>>> +++ b/drivers/pwm/pwm-mtk-disp.c
>>> @@ -18,33 +18,44 @@
>>>    #include <linux/io.h>
>>>    #include <linux/module.h>
>>>    #include <linux/of.h>
>>> +#include <linux/of_device.h>
>>>    #include <linux/platform_device.h>
>>>    #include <linux/pwm.h>
>>>    #include <linux/slab.h>
>>>
>>>    #define DISP_PWM_EN		0x00
>>> -#define PWM_ENABLE_MASK		BIT(0)
>>>
>>>    #define DISP_PWM_COMMIT		0x08
>>>    #define PWM_COMMIT_MASK		BIT(0)
>>>
>>> -#define DISP_PWM_CON_0		0x10
>>>    #define PWM_CLKDIV_SHIFT	16
>>>    #define PWM_CLKDIV_MAX		0x3ff
>>>    #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
>>>
>>> -#define DISP_PWM_CON_1		0x14
>>>    #define PWM_PERIOD_BIT_WIDTH	12
>>>    #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
>>>
>>>    #define PWM_HIGH_WIDTH_SHIFT	16
>>>    #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
>>>
>>> +#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
>>> +#define MT2701_PWM_BLS_DEBUG		0xb0
>>
>> Do we need to set magic 0x3 in the debug register? If so, this should be
>> part of mtk_pwm_data, just as con0_sel.
>
> Do you mean that add bls_debug into struct mtk_pwm_data,
> and set 0x3 in mt2701 data, 0x0 in mt8173 data?
>

Yes.

>>
>>> +#define MT2701_PWM_BLS_DEBUG_MASK	0x3
>>> +
>>> +struct mtk_pwm_data {
>>> +	unsigned int enable_bit;
>>> +	unsigned int con0;
>>> +	unsigned int con0_sel;
>>> +	unsigned int con1;
>>> +	bool have_commit_reg;
>>> +};
>>> +
>>>    struct mtk_disp_pwm {
>>>    	struct pwm_chip chip;
>>>    	struct clk *clk_main;
>>>    	struct clk *clk_mm;
>>>    	void __iomem *base;
>>> +	const struct mtk_pwm_data *data;
>>
>> Couldn't that just be the offset for the commit register and set it to
>> 0x0 if the commit register is not present. Right now, we suppose the
>> DISP_PWM_EN is at offset 0x0, so this should not be a problem.
>>
>
> Do you mean that replace have_commit_reg with commit_reg_offset?
> just like this?
> bool have_commit_reg;
> ->
> unsigned int commit_reg_offset;
> and set commit_reg_offset 0x0 in mt2701, 0x1 in mt8173?
>

No, you can set comit_reg_offset to the value of DISP_PWM_COMMIT for 
mt8173 and 0x0 for mt2701. Then you can get rid of DISP_PWM_COMMIT.

Regards,
Matthias

>>>    };
>>>
>>>    static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
>>> @@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>>>    		return err;
>>>    	}
>>>
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
>>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
>>> +				 PWM_CLKDIV_MASK,
>>>    				 clk_div << PWM_CLKDIV_SHIFT);
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
>>> -				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
>>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
>>> +				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
>>> +				 value);
>>> +	if (mdp->data->have_commit_reg) {
>>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
>>> +					 PWM_COMMIT_MASK, 0x1);
>>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
>>> +					 PWM_COMMIT_MASK, 0x0);
>>
>> should be renamed to MT8173_PWM_COMMIT_MASK, although I'm not really a
>> friend of mixing up SoC specific defines with a Soc specific
>> mtk_disp_pwm structure.
>>
>> Regards,
>> Matthias
>
> ok, I will modify it as you said. thanks for you suggestion.
>
>>
>>> +	}
>>>
>>>    	clk_disable(mdp->clk_mm);
>>>    	clk_disable(mdp->clk_main);
>>> @@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>>>    		return err;
>>>    	}
>>>
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
>>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
>>> +				 mdp->data->enable_bit,
>>> +				 mdp->data->enable_bit);
>>>
>>>    	return 0;
>>>    }
>>> @@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>>>    {
>>>    	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
>>>
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
>>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
>>> +				 mdp->data->enable_bit, 0x0);
>>>
>>>    	clk_disable(mdp->clk_mm);
>>>    	clk_disable(mdp->clk_main);
>>> @@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
>>>    	.owner = THIS_MODULE,
>>>    };
>>>
>>> +static const struct mtk_pwm_data mt8173_pwm_data = {
>>> +	.enable_bit = BIT(0),
>>> +	.con0 = 0x10,
>>> +	.con0_sel = 0x0,
>>> +	.con1 = 0x14,
>>> +	.have_commit_reg = true,
>>> +};
>>> +
>>> +static const struct mtk_pwm_data mt2701_pwm_data = {
>>> +	.enable_bit = BIT(16),
>>> +	.con0 = 0xa8,
>>> +	.con0_sel = 0x2,
>>> +	.con1 = 0xac,
>>> +	.have_commit_reg = false,
>>> +};
>>> +
>>> +static const struct of_device_id mtk_disp_pwm_of_match[] = {
>>> +	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
>>> +	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
>>> +	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
>>> +
>>>    static int mtk_disp_pwm_probe(struct platform_device *pdev)
>>>    {
>>> +	const struct of_device_id *id;
>>>    	struct mtk_disp_pwm *mdp;
>>>    	struct resource *r;
>>>    	int ret;
>>>
>>> +	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
>>> +	if (!id)
>>> +		return -EINVAL;
>>> +
>>>    	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
>>>    	if (!mdp)
>>>    		return -ENOMEM;
>>> @@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
>>>    	mdp->chip.ops = &mtk_disp_pwm_ops;
>>>    	mdp->chip.base = -1;
>>>    	mdp->chip.npwm = 1;
>>> +	mdp->data = id->data;
>>>
>>>    	ret = pwmchip_add(&mdp->chip);
>>>    	if (ret < 0) {
>>> @@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
>>>
>>>    	platform_set_drvdata(pdev, mdp);
>>>
>>> +	/*
>>> +	 * For MT2701, disable double buffer before writing register
>>> +	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
>>> +	 */
>>> +	if (!mdp->data->have_commit_reg) {
>>> +		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
>>> +					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
>>> +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
>>> +					 MT2701_PWM_MANUAL_SEL_MASK,
>>> +					 mdp->data->con0_sel);
>>> +	}
>>> +
>>>    	return 0;
>>>
>>>    disable_clk_mm:
>>> @@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
>>>    	return ret;
>>>    }
>>>
>>> -static const struct of_device_id mtk_disp_pwm_of_match[] = {
>>> -	{ .compatible = "mediatek,mt8173-disp-pwm" },
>>> -	{ .compatible = "mediatek,mt6595-disp-pwm" },
>>> -	{ }
>>> -};
>>> -MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
>>> -
>>>    static struct platform_driver mtk_disp_pwm_driver = {
>>>    	.driver = {
>>>    		.name = "mediatek-disp-pwm",
>>>
>
>
--
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

Re: [PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[Matthias Brugger]

On 08/06/16 09:21, weiqing kong wrote:
> On Fri, 2016-06-03 at 17:35 +0200, Matthias Brugger wrote:
>>
>> On 30/05/16 10:41, Weiqing Kong wrote:
>>> Use the mtk_pwm_data struction to define different registers
>>> and add MT2701 specific register operations, such as MT2701
>>> doesn't have commit register, needs to disable double buffer
>>> before writing register, and needs to select manual mode
>>> and use PWM_PERIOD/PWM_HIGH_WIDTH.
>>>
>>> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
>>> ---
>>>    drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
>>>    1 file changed, 72 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
>>> index 0ad3385..03b9c9e 100644
>>> --- a/drivers/pwm/pwm-mtk-disp.c
>>> +++ b/drivers/pwm/pwm-mtk-disp.c
>>> @@ -18,33 +18,44 @@
>>>    #include <linux/io.h>
>>>    #include <linux/module.h>
>>>    #include <linux/of.h>
>>> +#include <linux/of_device.h>
>>>    #include <linux/platform_device.h>
>>>    #include <linux/pwm.h>
>>>    #include <linux/slab.h>
>>>
>>>    #define DISP_PWM_EN		0x00
>>> -#define PWM_ENABLE_MASK		BIT(0)
>>>
>>>    #define DISP_PWM_COMMIT		0x08
>>>    #define PWM_COMMIT_MASK		BIT(0)
>>>
>>> -#define DISP_PWM_CON_0		0x10
>>>    #define PWM_CLKDIV_SHIFT	16
>>>    #define PWM_CLKDIV_MAX		0x3ff
>>>    #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
>>>
>>> -#define DISP_PWM_CON_1		0x14
>>>    #define PWM_PERIOD_BIT_WIDTH	12
>>>    #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
>>>
>>>    #define PWM_HIGH_WIDTH_SHIFT	16
>>>    #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
>>>
>>> +#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
>>> +#define MT2701_PWM_BLS_DEBUG		0xb0
>>
>> Do we need to set magic 0x3 in the debug register? If so, this should be
>> part of mtk_pwm_data, just as con0_sel.
>
> Do you mean that add bls_debug into struct mtk_pwm_data,
> and set 0x3 in mt2701 data, 0x0 in mt8173 data?
>

Yes.

>>
>>> +#define MT2701_PWM_BLS_DEBUG_MASK	0x3
>>> +
>>> +struct mtk_pwm_data {
>>> +	unsigned int enable_bit;
>>> +	unsigned int con0;
>>> +	unsigned int con0_sel;
>>> +	unsigned int con1;
>>> +	bool have_commit_reg;
>>> +};
>>> +
>>>    struct mtk_disp_pwm {
>>>    	struct pwm_chip chip;
>>>    	struct clk *clk_main;
>>>    	struct clk *clk_mm;
>>>    	void __iomem *base;
>>> +	const struct mtk_pwm_data *data;
>>
>> Couldn't that just be the offset for the commit register and set it to
>> 0x0 if the commit register is not present. Right now, we suppose the
>> DISP_PWM_EN is at offset 0x0, so this should not be a problem.
>>
>
> Do you mean that replace have_commit_reg with commit_reg_offset?
> just like this?
> bool have_commit_reg;
> ->
> unsigned int commit_reg_offset;
> and set commit_reg_offset 0x0 in mt2701, 0x1 in mt8173?
>

No, you can set comit_reg_offset to the value of DISP_PWM_COMMIT for 
mt8173 and 0x0 for mt2701. Then you can get rid of DISP_PWM_COMMIT.

Regards,
Matthias

>>>    };
>>>
>>>    static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
>>> @@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>>>    		return err;
>>>    	}
>>>
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
>>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
>>> +				 PWM_CLKDIV_MASK,
>>>    				 clk_div << PWM_CLKDIV_SHIFT);
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
>>> -				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
>>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
>>> +				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
>>> +				 value);
>>> +	if (mdp->data->have_commit_reg) {
>>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
>>> +					 PWM_COMMIT_MASK, 0x1);
>>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
>>> +					 PWM_COMMIT_MASK, 0x0);
>>
>> should be renamed to MT8173_PWM_COMMIT_MASK, although I'm not really a
>> friend of mixing up SoC specific defines with a Soc specific
>> mtk_disp_pwm structure.
>>
>> Regards,
>> Matthias
>
> ok, I will modify it as you said. thanks for you suggestion.
>
>>
>>> +	}
>>>
>>>    	clk_disable(mdp->clk_mm);
>>>    	clk_disable(mdp->clk_main);
>>> @@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>>>    		return err;
>>>    	}
>>>
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
>>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
>>> +				 mdp->data->enable_bit,
>>> +				 mdp->data->enable_bit);
>>>
>>>    	return 0;
>>>    }
>>> @@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>>>    {
>>>    	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
>>>
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
>>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
>>> +				 mdp->data->enable_bit, 0x0);
>>>
>>>    	clk_disable(mdp->clk_mm);
>>>    	clk_disable(mdp->clk_main);
>>> @@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
>>>    	.owner = THIS_MODULE,
>>>    };
>>>
>>> +static const struct mtk_pwm_data mt8173_pwm_data = {
>>> +	.enable_bit = BIT(0),
>>> +	.con0 = 0x10,
>>> +	.con0_sel = 0x0,
>>> +	.con1 = 0x14,
>>> +	.have_commit_reg = true,
>>> +};
>>> +
>>> +static const struct mtk_pwm_data mt2701_pwm_data = {
>>> +	.enable_bit = BIT(16),
>>> +	.con0 = 0xa8,
>>> +	.con0_sel = 0x2,
>>> +	.con1 = 0xac,
>>> +	.have_commit_reg = false,
>>> +};
>>> +
>>> +static const struct of_device_id mtk_disp_pwm_of_match[] = {
>>> +	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
>>> +	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
>>> +	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
>>> +
>>>    static int mtk_disp_pwm_probe(struct platform_device *pdev)
>>>    {
>>> +	const struct of_device_id *id;
>>>    	struct mtk_disp_pwm *mdp;
>>>    	struct resource *r;
>>>    	int ret;
>>>
>>> +	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
>>> +	if (!id)
>>> +		return -EINVAL;
>>> +
>>>    	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
>>>    	if (!mdp)
>>>    		return -ENOMEM;
>>> @@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
>>>    	mdp->chip.ops = &mtk_disp_pwm_ops;
>>>    	mdp->chip.base = -1;
>>>    	mdp->chip.npwm = 1;
>>> +	mdp->data = id->data;
>>>
>>>    	ret = pwmchip_add(&mdp->chip);
>>>    	if (ret < 0) {
>>> @@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
>>>
>>>    	platform_set_drvdata(pdev, mdp);
>>>
>>> +	/*
>>> +	 * For MT2701, disable double buffer before writing register
>>> +	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
>>> +	 */
>>> +	if (!mdp->data->have_commit_reg) {
>>> +		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
>>> +					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
>>> +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
>>> +					 MT2701_PWM_MANUAL_SEL_MASK,
>>> +					 mdp->data->con0_sel);
>>> +	}
>>> +
>>>    	return 0;
>>>
>>>    disable_clk_mm:
>>> @@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
>>>    	return ret;
>>>    }
>>>
>>> -static const struct of_device_id mtk_disp_pwm_of_match[] = {
>>> -	{ .compatible = "mediatek,mt8173-disp-pwm" },
>>> -	{ .compatible = "mediatek,mt6595-disp-pwm" },
>>> -	{ }
>>> -};
>>> -MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
>>> -
>>>    static struct platform_driver mtk_disp_pwm_driver = {
>>>    	.driver = {
>>>    		.name = "mediatek-disp-pwm",
>>>
>
>

[PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[Matthias Brugger]

On 08/06/16 09:21, weiqing kong wrote:
> On Fri, 2016-06-03 at 17:35 +0200, Matthias Brugger wrote:
>>
>> On 30/05/16 10:41, Weiqing Kong wrote:
>>> Use the mtk_pwm_data struction to define different registers
>>> and add MT2701 specific register operations, such as MT2701
>>> doesn't have commit register, needs to disable double buffer
>>> before writing register, and needs to select manual mode
>>> and use PWM_PERIOD/PWM_HIGH_WIDTH.
>>>
>>> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
>>> ---
>>>    drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
>>>    1 file changed, 72 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
>>> index 0ad3385..03b9c9e 100644
>>> --- a/drivers/pwm/pwm-mtk-disp.c
>>> +++ b/drivers/pwm/pwm-mtk-disp.c
>>> @@ -18,33 +18,44 @@
>>>    #include <linux/io.h>
>>>    #include <linux/module.h>
>>>    #include <linux/of.h>
>>> +#include <linux/of_device.h>
>>>    #include <linux/platform_device.h>
>>>    #include <linux/pwm.h>
>>>    #include <linux/slab.h>
>>>
>>>    #define DISP_PWM_EN		0x00
>>> -#define PWM_ENABLE_MASK		BIT(0)
>>>
>>>    #define DISP_PWM_COMMIT		0x08
>>>    #define PWM_COMMIT_MASK		BIT(0)
>>>
>>> -#define DISP_PWM_CON_0		0x10
>>>    #define PWM_CLKDIV_SHIFT	16
>>>    #define PWM_CLKDIV_MAX		0x3ff
>>>    #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
>>>
>>> -#define DISP_PWM_CON_1		0x14
>>>    #define PWM_PERIOD_BIT_WIDTH	12
>>>    #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
>>>
>>>    #define PWM_HIGH_WIDTH_SHIFT	16
>>>    #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
>>>
>>> +#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
>>> +#define MT2701_PWM_BLS_DEBUG		0xb0
>>
>> Do we need to set magic 0x3 in the debug register? If so, this should be
>> part of mtk_pwm_data, just as con0_sel.
>
> Do you mean that add bls_debug into struct mtk_pwm_data,
> and set 0x3 in mt2701 data, 0x0 in mt8173 data?
>

Yes.

>>
>>> +#define MT2701_PWM_BLS_DEBUG_MASK	0x3
>>> +
>>> +struct mtk_pwm_data {
>>> +	unsigned int enable_bit;
>>> +	unsigned int con0;
>>> +	unsigned int con0_sel;
>>> +	unsigned int con1;
>>> +	bool have_commit_reg;
>>> +};
>>> +
>>>    struct mtk_disp_pwm {
>>>    	struct pwm_chip chip;
>>>    	struct clk *clk_main;
>>>    	struct clk *clk_mm;
>>>    	void __iomem *base;
>>> +	const struct mtk_pwm_data *data;
>>
>> Couldn't that just be the offset for the commit register and set it to
>> 0x0 if the commit register is not present. Right now, we suppose the
>> DISP_PWM_EN is at offset 0x0, so this should not be a problem.
>>
>
> Do you mean that replace have_commit_reg with commit_reg_offset?
> just like this?
> bool have_commit_reg;
> ->
> unsigned int commit_reg_offset;
> and set commit_reg_offset 0x0 in mt2701, 0x1 in mt8173?
>

No, you can set comit_reg_offset to the value of DISP_PWM_COMMIT for 
mt8173 and 0x0 for mt2701. Then you can get rid of DISP_PWM_COMMIT.

Regards,
Matthias

>>>    };
>>>
>>>    static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
>>> @@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>>>    		return err;
>>>    	}
>>>
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
>>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
>>> +				 PWM_CLKDIV_MASK,
>>>    				 clk_div << PWM_CLKDIV_SHIFT);
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
>>> -				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
>>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
>>> +				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
>>> +				 value);
>>> +	if (mdp->data->have_commit_reg) {
>>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
>>> +					 PWM_COMMIT_MASK, 0x1);
>>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
>>> +					 PWM_COMMIT_MASK, 0x0);
>>
>> should be renamed to MT8173_PWM_COMMIT_MASK, although I'm not really a
>> friend of mixing up SoC specific defines with a Soc specific
>> mtk_disp_pwm structure.
>>
>> Regards,
>> Matthias
>
> ok, I will modify it as you said. thanks for you suggestion.
>
>>
>>> +	}
>>>
>>>    	clk_disable(mdp->clk_mm);
>>>    	clk_disable(mdp->clk_main);
>>> @@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>>>    		return err;
>>>    	}
>>>
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
>>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
>>> +				 mdp->data->enable_bit,
>>> +				 mdp->data->enable_bit);
>>>
>>>    	return 0;
>>>    }
>>> @@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>>>    {
>>>    	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
>>>
>>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
>>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
>>> +				 mdp->data->enable_bit, 0x0);
>>>
>>>    	clk_disable(mdp->clk_mm);
>>>    	clk_disable(mdp->clk_main);
>>> @@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
>>>    	.owner = THIS_MODULE,
>>>    };
>>>
>>> +static const struct mtk_pwm_data mt8173_pwm_data = {
>>> +	.enable_bit = BIT(0),
>>> +	.con0 = 0x10,
>>> +	.con0_sel = 0x0,
>>> +	.con1 = 0x14,
>>> +	.have_commit_reg = true,
>>> +};
>>> +
>>> +static const struct mtk_pwm_data mt2701_pwm_data = {
>>> +	.enable_bit = BIT(16),
>>> +	.con0 = 0xa8,
>>> +	.con0_sel = 0x2,
>>> +	.con1 = 0xac,
>>> +	.have_commit_reg = false,
>>> +};
>>> +
>>> +static const struct of_device_id mtk_disp_pwm_of_match[] = {
>>> +	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
>>> +	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
>>> +	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
>>> +
>>>    static int mtk_disp_pwm_probe(struct platform_device *pdev)
>>>    {
>>> +	const struct of_device_id *id;
>>>    	struct mtk_disp_pwm *mdp;
>>>    	struct resource *r;
>>>    	int ret;
>>>
>>> +	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
>>> +	if (!id)
>>> +		return -EINVAL;
>>> +
>>>    	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
>>>    	if (!mdp)
>>>    		return -ENOMEM;
>>> @@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
>>>    	mdp->chip.ops = &mtk_disp_pwm_ops;
>>>    	mdp->chip.base = -1;
>>>    	mdp->chip.npwm = 1;
>>> +	mdp->data = id->data;
>>>
>>>    	ret = pwmchip_add(&mdp->chip);
>>>    	if (ret < 0) {
>>> @@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
>>>
>>>    	platform_set_drvdata(pdev, mdp);
>>>
>>> +	/*
>>> +	 * For MT2701, disable double buffer before writing register
>>> +	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
>>> +	 */
>>> +	if (!mdp->data->have_commit_reg) {
>>> +		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
>>> +					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
>>> +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
>>> +					 MT2701_PWM_MANUAL_SEL_MASK,
>>> +					 mdp->data->con0_sel);
>>> +	}
>>> +
>>>    	return 0;
>>>
>>>    disable_clk_mm:
>>> @@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
>>>    	return ret;
>>>    }
>>>
>>> -static const struct of_device_id mtk_disp_pwm_of_match[] = {
>>> -	{ .compatible = "mediatek,mt8173-disp-pwm" },
>>> -	{ .compatible = "mediatek,mt6595-disp-pwm" },
>>> -	{ }
>>> -};
>>> -MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
>>> -
>>>    static struct platform_driver mtk_disp_pwm_driver = {
>>>    	.driver = {
>>>    		.name = "mediatek-disp-pwm",
>>>
>
>

Re: [PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[weiqing kong]

On Wed, 2016-06-08 at 13:22 +0200, Matthias Brugger wrote:
> 
> On 08/06/16 09:21, weiqing kong wrote:
> > On Fri, 2016-06-03 at 17:35 +0200, Matthias Brugger wrote:
> >>
> >> On 30/05/16 10:41, Weiqing Kong wrote:
> >>> Use the mtk_pwm_data struction to define different registers
> >>> and add MT2701 specific register operations, such as MT2701
> >>> doesn't have commit register, needs to disable double buffer
> >>> before writing register, and needs to select manual mode
> >>> and use PWM_PERIOD/PWM_HIGH_WIDTH.
> >>>
> >>> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> >>> ---
> >>>    drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
> >>>    1 file changed, 72 insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> >>> index 0ad3385..03b9c9e 100644
> >>> --- a/drivers/pwm/pwm-mtk-disp.c
> >>> +++ b/drivers/pwm/pwm-mtk-disp.c
> >>> @@ -18,33 +18,44 @@
> >>>    #include <linux/io.h>
> >>>    #include <linux/module.h>
> >>>    #include <linux/of.h>
> >>> +#include <linux/of_device.h>
> >>>    #include <linux/platform_device.h>
> >>>    #include <linux/pwm.h>
> >>>    #include <linux/slab.h>
> >>>
> >>>    #define DISP_PWM_EN		0x00
> >>> -#define PWM_ENABLE_MASK		BIT(0)
> >>>
> >>>    #define DISP_PWM_COMMIT		0x08
> >>>    #define PWM_COMMIT_MASK		BIT(0)
> >>>
> >>> -#define DISP_PWM_CON_0		0x10
> >>>    #define PWM_CLKDIV_SHIFT	16
> >>>    #define PWM_CLKDIV_MAX		0x3ff
> >>>    #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
> >>>
> >>> -#define DISP_PWM_CON_1		0x14
> >>>    #define PWM_PERIOD_BIT_WIDTH	12
> >>>    #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
> >>>
> >>>    #define PWM_HIGH_WIDTH_SHIFT	16
> >>>    #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
> >>>
> >>> +#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
> >>> +#define MT2701_PWM_BLS_DEBUG		0xb0
> >>
> >> Do we need to set magic 0x3 in the debug register? If so, this should be
> >> part of mtk_pwm_data, just as con0_sel.
> >
> > Do you mean that add bls_debug into struct mtk_pwm_data,
> > and set 0x3 in mt2701 data, 0x0 in mt8173 data?
> >
> 
> Yes.
> 
> >>
> >>> +#define MT2701_PWM_BLS_DEBUG_MASK	0x3
> >>> +
> >>> +struct mtk_pwm_data {
> >>> +	unsigned int enable_bit;
> >>> +	unsigned int con0;
> >>> +	unsigned int con0_sel;
> >>> +	unsigned int con1;
> >>> +	bool have_commit_reg;
> >>> +};
> >>> +
> >>>    struct mtk_disp_pwm {
> >>>    	struct pwm_chip chip;
> >>>    	struct clk *clk_main;
> >>>    	struct clk *clk_mm;
> >>>    	void __iomem *base;
> >>> +	const struct mtk_pwm_data *data;
> >>
> >> Couldn't that just be the offset for the commit register and set it to
> >> 0x0 if the commit register is not present. Right now, we suppose the
> >> DISP_PWM_EN is at offset 0x0, so this should not be a problem.
> >>
> >
> > Do you mean that replace have_commit_reg with commit_reg_offset?
> > just like this?
> > bool have_commit_reg;
> > ->
> > unsigned int commit_reg_offset;
> > and set commit_reg_offset 0x0 in mt2701, 0x1 in mt8173?
> >
> 
> No, you can set comit_reg_offset to the value of DISP_PWM_COMMIT for 
> mt8173 and 0x0 for mt2701. Then you can get rid of DISP_PWM_COMMIT.
> 
> Regards,
> Matthias

ok, I got it, thanks.

> 
> >>>    };
> >>>
> >>>    static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
> >>> @@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >>>    		return err;
> >>>    	}
> >>>
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
> >>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> >>> +				 PWM_CLKDIV_MASK,
> >>>    				 clk_div << PWM_CLKDIV_SHIFT);
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
> >>> -				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
> >>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
> >>> +				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
> >>> +				 value);
> >>> +	if (mdp->data->have_commit_reg) {
> >>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> >>> +					 PWM_COMMIT_MASK, 0x1);
> >>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> >>> +					 PWM_COMMIT_MASK, 0x0);
> >>
> >> should be renamed to MT8173_PWM_COMMIT_MASK, although I'm not really a
> >> friend of mixing up SoC specific defines with a Soc specific
> >> mtk_disp_pwm structure.
> >>
> >> Regards,
> >> Matthias
> >
> > ok, I will modify it as you said. thanks for you suggestion.
> >
> >>
> >>> +	}
> >>>
> >>>    	clk_disable(mdp->clk_mm);
> >>>    	clk_disable(mdp->clk_main);
> >>> @@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> >>>    		return err;
> >>>    	}
> >>>
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
> >>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> >>> +				 mdp->data->enable_bit,
> >>> +				 mdp->data->enable_bit);
> >>>
> >>>    	return 0;
> >>>    }
> >>> @@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >>>    {
> >>>    	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
> >>>
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
> >>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> >>> +				 mdp->data->enable_bit, 0x0);
> >>>
> >>>    	clk_disable(mdp->clk_mm);
> >>>    	clk_disable(mdp->clk_main);
> >>> @@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
> >>>    	.owner = THIS_MODULE,
> >>>    };
> >>>
> >>> +static const struct mtk_pwm_data mt8173_pwm_data = {
> >>> +	.enable_bit = BIT(0),
> >>> +	.con0 = 0x10,
> >>> +	.con0_sel = 0x0,
> >>> +	.con1 = 0x14,
> >>> +	.have_commit_reg = true,
> >>> +};
> >>> +
> >>> +static const struct mtk_pwm_data mt2701_pwm_data = {
> >>> +	.enable_bit = BIT(16),
> >>> +	.con0 = 0xa8,
> >>> +	.con0_sel = 0x2,
> >>> +	.con1 = 0xac,
> >>> +	.have_commit_reg = false,
> >>> +};
> >>> +
> >>> +static const struct of_device_id mtk_disp_pwm_of_match[] = {
> >>> +	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
> >>> +	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
> >>> +	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
> >>> +	{ }
> >>> +};
> >>> +MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> >>> +
> >>>    static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >>>    {
> >>> +	const struct of_device_id *id;
> >>>    	struct mtk_disp_pwm *mdp;
> >>>    	struct resource *r;
> >>>    	int ret;
> >>>
> >>> +	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
> >>> +	if (!id)
> >>> +		return -EINVAL;
> >>> +
> >>>    	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
> >>>    	if (!mdp)
> >>>    		return -ENOMEM;
> >>> @@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >>>    	mdp->chip.ops = &mtk_disp_pwm_ops;
> >>>    	mdp->chip.base = -1;
> >>>    	mdp->chip.npwm = 1;
> >>> +	mdp->data = id->data;
> >>>
> >>>    	ret = pwmchip_add(&mdp->chip);
> >>>    	if (ret < 0) {
> >>> @@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >>>
> >>>    	platform_set_drvdata(pdev, mdp);
> >>>
> >>> +	/*
> >>> +	 * For MT2701, disable double buffer before writing register
> >>> +	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
> >>> +	 */
> >>> +	if (!mdp->data->have_commit_reg) {
> >>> +		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
> >>> +					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
> >>> +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> >>> +					 MT2701_PWM_MANUAL_SEL_MASK,
> >>> +					 mdp->data->con0_sel);
> >>> +	}
> >>> +
> >>>    	return 0;
> >>>
> >>>    disable_clk_mm:
> >>> @@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
> >>>    	return ret;
> >>>    }
> >>>
> >>> -static const struct of_device_id mtk_disp_pwm_of_match[] = {
> >>> -	{ .compatible = "mediatek,mt8173-disp-pwm" },
> >>> -	{ .compatible = "mediatek,mt6595-disp-pwm" },
> >>> -	{ }
> >>> -};
> >>> -MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> >>> -
> >>>    static struct platform_driver mtk_disp_pwm_driver = {
> >>>    	.driver = {
> >>>    		.name = "mediatek-disp-pwm",
> >>>
> >
> >

Re: [PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[weiqing kong]

On Wed, 2016-06-08 at 13:22 +0200, Matthias Brugger wrote:
> 
> On 08/06/16 09:21, weiqing kong wrote:
> > On Fri, 2016-06-03 at 17:35 +0200, Matthias Brugger wrote:
> >>
> >> On 30/05/16 10:41, Weiqing Kong wrote:
> >>> Use the mtk_pwm_data struction to define different registers
> >>> and add MT2701 specific register operations, such as MT2701
> >>> doesn't have commit register, needs to disable double buffer
> >>> before writing register, and needs to select manual mode
> >>> and use PWM_PERIOD/PWM_HIGH_WIDTH.
> >>>
> >>> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> >>> ---
> >>>    drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
> >>>    1 file changed, 72 insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> >>> index 0ad3385..03b9c9e 100644
> >>> --- a/drivers/pwm/pwm-mtk-disp.c
> >>> +++ b/drivers/pwm/pwm-mtk-disp.c
> >>> @@ -18,33 +18,44 @@
> >>>    #include <linux/io.h>
> >>>    #include <linux/module.h>
> >>>    #include <linux/of.h>
> >>> +#include <linux/of_device.h>
> >>>    #include <linux/platform_device.h>
> >>>    #include <linux/pwm.h>
> >>>    #include <linux/slab.h>
> >>>
> >>>    #define DISP_PWM_EN		0x00
> >>> -#define PWM_ENABLE_MASK		BIT(0)
> >>>
> >>>    #define DISP_PWM_COMMIT		0x08
> >>>    #define PWM_COMMIT_MASK		BIT(0)
> >>>
> >>> -#define DISP_PWM_CON_0		0x10
> >>>    #define PWM_CLKDIV_SHIFT	16
> >>>    #define PWM_CLKDIV_MAX		0x3ff
> >>>    #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
> >>>
> >>> -#define DISP_PWM_CON_1		0x14
> >>>    #define PWM_PERIOD_BIT_WIDTH	12
> >>>    #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
> >>>
> >>>    #define PWM_HIGH_WIDTH_SHIFT	16
> >>>    #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
> >>>
> >>> +#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
> >>> +#define MT2701_PWM_BLS_DEBUG		0xb0
> >>
> >> Do we need to set magic 0x3 in the debug register? If so, this should be
> >> part of mtk_pwm_data, just as con0_sel.
> >
> > Do you mean that add bls_debug into struct mtk_pwm_data,
> > and set 0x3 in mt2701 data, 0x0 in mt8173 data?
> >
> 
> Yes.
> 
> >>
> >>> +#define MT2701_PWM_BLS_DEBUG_MASK	0x3
> >>> +
> >>> +struct mtk_pwm_data {
> >>> +	unsigned int enable_bit;
> >>> +	unsigned int con0;
> >>> +	unsigned int con0_sel;
> >>> +	unsigned int con1;
> >>> +	bool have_commit_reg;
> >>> +};
> >>> +
> >>>    struct mtk_disp_pwm {
> >>>    	struct pwm_chip chip;
> >>>    	struct clk *clk_main;
> >>>    	struct clk *clk_mm;
> >>>    	void __iomem *base;
> >>> +	const struct mtk_pwm_data *data;
> >>
> >> Couldn't that just be the offset for the commit register and set it to
> >> 0x0 if the commit register is not present. Right now, we suppose the
> >> DISP_PWM_EN is at offset 0x0, so this should not be a problem.
> >>
> >
> > Do you mean that replace have_commit_reg with commit_reg_offset?
> > just like this?
> > bool have_commit_reg;
> > ->
> > unsigned int commit_reg_offset;
> > and set commit_reg_offset 0x0 in mt2701, 0x1 in mt8173?
> >
> 
> No, you can set comit_reg_offset to the value of DISP_PWM_COMMIT for 
> mt8173 and 0x0 for mt2701. Then you can get rid of DISP_PWM_COMMIT.
> 
> Regards,
> Matthias

ok, I got it, thanks.

> 
> >>>    };
> >>>
> >>>    static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
> >>> @@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >>>    		return err;
> >>>    	}
> >>>
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
> >>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> >>> +				 PWM_CLKDIV_MASK,
> >>>    				 clk_div << PWM_CLKDIV_SHIFT);
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
> >>> -				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
> >>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
> >>> +				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
> >>> +				 value);
> >>> +	if (mdp->data->have_commit_reg) {
> >>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> >>> +					 PWM_COMMIT_MASK, 0x1);
> >>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> >>> +					 PWM_COMMIT_MASK, 0x0);
> >>
> >> should be renamed to MT8173_PWM_COMMIT_MASK, although I'm not really a
> >> friend of mixing up SoC specific defines with a Soc specific
> >> mtk_disp_pwm structure.
> >>
> >> Regards,
> >> Matthias
> >
> > ok, I will modify it as you said. thanks for you suggestion.
> >
> >>
> >>> +	}
> >>>
> >>>    	clk_disable(mdp->clk_mm);
> >>>    	clk_disable(mdp->clk_main);
> >>> @@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> >>>    		return err;
> >>>    	}
> >>>
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
> >>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> >>> +				 mdp->data->enable_bit,
> >>> +				 mdp->data->enable_bit);
> >>>
> >>>    	return 0;
> >>>    }
> >>> @@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >>>    {
> >>>    	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
> >>>
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
> >>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> >>> +				 mdp->data->enable_bit, 0x0);
> >>>
> >>>    	clk_disable(mdp->clk_mm);
> >>>    	clk_disable(mdp->clk_main);
> >>> @@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
> >>>    	.owner = THIS_MODULE,
> >>>    };
> >>>
> >>> +static const struct mtk_pwm_data mt8173_pwm_data = {
> >>> +	.enable_bit = BIT(0),
> >>> +	.con0 = 0x10,
> >>> +	.con0_sel = 0x0,
> >>> +	.con1 = 0x14,
> >>> +	.have_commit_reg = true,
> >>> +};
> >>> +
> >>> +static const struct mtk_pwm_data mt2701_pwm_data = {
> >>> +	.enable_bit = BIT(16),
> >>> +	.con0 = 0xa8,
> >>> +	.con0_sel = 0x2,
> >>> +	.con1 = 0xac,
> >>> +	.have_commit_reg = false,
> >>> +};
> >>> +
> >>> +static const struct of_device_id mtk_disp_pwm_of_match[] = {
> >>> +	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
> >>> +	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
> >>> +	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
> >>> +	{ }
> >>> +};
> >>> +MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> >>> +
> >>>    static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >>>    {
> >>> +	const struct of_device_id *id;
> >>>    	struct mtk_disp_pwm *mdp;
> >>>    	struct resource *r;
> >>>    	int ret;
> >>>
> >>> +	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
> >>> +	if (!id)
> >>> +		return -EINVAL;
> >>> +
> >>>    	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
> >>>    	if (!mdp)
> >>>    		return -ENOMEM;
> >>> @@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >>>    	mdp->chip.ops = &mtk_disp_pwm_ops;
> >>>    	mdp->chip.base = -1;
> >>>    	mdp->chip.npwm = 1;
> >>> +	mdp->data = id->data;
> >>>
> >>>    	ret = pwmchip_add(&mdp->chip);
> >>>    	if (ret < 0) {
> >>> @@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >>>
> >>>    	platform_set_drvdata(pdev, mdp);
> >>>
> >>> +	/*
> >>> +	 * For MT2701, disable double buffer before writing register
> >>> +	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
> >>> +	 */
> >>> +	if (!mdp->data->have_commit_reg) {
> >>> +		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
> >>> +					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
> >>> +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> >>> +					 MT2701_PWM_MANUAL_SEL_MASK,
> >>> +					 mdp->data->con0_sel);
> >>> +	}
> >>> +
> >>>    	return 0;
> >>>
> >>>    disable_clk_mm:
> >>> @@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
> >>>    	return ret;
> >>>    }
> >>>
> >>> -static const struct of_device_id mtk_disp_pwm_of_match[] = {
> >>> -	{ .compatible = "mediatek,mt8173-disp-pwm" },
> >>> -	{ .compatible = "mediatek,mt6595-disp-pwm" },
> >>> -	{ }
> >>> -};
> >>> -MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> >>> -
> >>>    static struct platform_driver mtk_disp_pwm_driver = {
> >>>    	.driver = {
> >>>    		.name = "mediatek-disp-pwm",
> >>>
> >
> >

[PATCH 2/3] pwm: Add MediaTek MT2701 display PWM driver support

[weiqing kong]

On Wed, 2016-06-08 at 13:22 +0200, Matthias Brugger wrote:
> 
> On 08/06/16 09:21, weiqing kong wrote:
> > On Fri, 2016-06-03 at 17:35 +0200, Matthias Brugger wrote:
> >>
> >> On 30/05/16 10:41, Weiqing Kong wrote:
> >>> Use the mtk_pwm_data struction to define different registers
> >>> and add MT2701 specific register operations, such as MT2701
> >>> doesn't have commit register, needs to disable double buffer
> >>> before writing register, and needs to select manual mode
> >>> and use PWM_PERIOD/PWM_HIGH_WIDTH.
> >>>
> >>> Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
> >>> ---
> >>>    drivers/pwm/pwm-mtk-disp.c | 89 +++++++++++++++++++++++++++++++++++++---------
> >>>    1 file changed, 72 insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/drivers/pwm/pwm-mtk-disp.c b/drivers/pwm/pwm-mtk-disp.c
> >>> index 0ad3385..03b9c9e 100644
> >>> --- a/drivers/pwm/pwm-mtk-disp.c
> >>> +++ b/drivers/pwm/pwm-mtk-disp.c
> >>> @@ -18,33 +18,44 @@
> >>>    #include <linux/io.h>
> >>>    #include <linux/module.h>
> >>>    #include <linux/of.h>
> >>> +#include <linux/of_device.h>
> >>>    #include <linux/platform_device.h>
> >>>    #include <linux/pwm.h>
> >>>    #include <linux/slab.h>
> >>>
> >>>    #define DISP_PWM_EN		0x00
> >>> -#define PWM_ENABLE_MASK		BIT(0)
> >>>
> >>>    #define DISP_PWM_COMMIT		0x08
> >>>    #define PWM_COMMIT_MASK		BIT(0)
> >>>
> >>> -#define DISP_PWM_CON_0		0x10
> >>>    #define PWM_CLKDIV_SHIFT	16
> >>>    #define PWM_CLKDIV_MAX		0x3ff
> >>>    #define PWM_CLKDIV_MASK		(PWM_CLKDIV_MAX << PWM_CLKDIV_SHIFT)
> >>>
> >>> -#define DISP_PWM_CON_1		0x14
> >>>    #define PWM_PERIOD_BIT_WIDTH	12
> >>>    #define PWM_PERIOD_MASK		((1 << PWM_PERIOD_BIT_WIDTH) - 1)
> >>>
> >>>    #define PWM_HIGH_WIDTH_SHIFT	16
> >>>    #define PWM_HIGH_WIDTH_MASK	(0x1fff << PWM_HIGH_WIDTH_SHIFT)
> >>>
> >>> +#define MT2701_PWM_MANUAL_SEL_MASK	BIT(1)
> >>> +#define MT2701_PWM_BLS_DEBUG		0xb0
> >>
> >> Do we need to set magic 0x3 in the debug register? If so, this should be
> >> part of mtk_pwm_data, just as con0_sel.
> >
> > Do you mean that add bls_debug into struct mtk_pwm_data,
> > and set 0x3 in mt2701 data, 0x0 in mt8173 data?
> >
> 
> Yes.
> 
> >>
> >>> +#define MT2701_PWM_BLS_DEBUG_MASK	0x3
> >>> +
> >>> +struct mtk_pwm_data {
> >>> +	unsigned int enable_bit;
> >>> +	unsigned int con0;
> >>> +	unsigned int con0_sel;
> >>> +	unsigned int con1;
> >>> +	bool have_commit_reg;
> >>> +};
> >>> +
> >>>    struct mtk_disp_pwm {
> >>>    	struct pwm_chip chip;
> >>>    	struct clk *clk_main;
> >>>    	struct clk *clk_mm;
> >>>    	void __iomem *base;
> >>> +	const struct mtk_pwm_data *data;
> >>
> >> Couldn't that just be the offset for the commit register and set it to
> >> 0x0 if the commit register is not present. Right now, we suppose the
> >> DISP_PWM_EN is at offset 0x0, so this should not be a problem.
> >>
> >
> > Do you mean that replace have_commit_reg with commit_reg_offset?
> > just like this?
> > bool have_commit_reg;
> > ->
> > unsigned int commit_reg_offset;
> > and set commit_reg_offset 0x0 in mt2701, 0x1 in mt8173?
> >
> 
> No, you can set comit_reg_offset to the value of DISP_PWM_COMMIT for 
> mt8173 and 0x0 for mt2701. Then you can get rid of DISP_PWM_COMMIT.
> 
> Regards,
> Matthias

ok, I got it, thanks.

> 
> >>>    };
> >>>
> >>>    static inline struct mtk_disp_pwm *to_mtk_disp_pwm(struct pwm_chip *chip)
> >>> @@ -106,12 +117,18 @@ static int mtk_disp_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >>>    		return err;
> >>>    	}
> >>>
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_0, PWM_CLKDIV_MASK,
> >>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> >>> +				 PWM_CLKDIV_MASK,
> >>>    				 clk_div << PWM_CLKDIV_SHIFT);
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_CON_1,
> >>> -				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK, value);
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 1);
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT, PWM_COMMIT_MASK, 0);
> >>> +	mtk_disp_pwm_update_bits(mdp, mdp->data->con1,
> >>> +				 PWM_PERIOD_MASK | PWM_HIGH_WIDTH_MASK,
> >>> +				 value);
> >>> +	if (mdp->data->have_commit_reg) {
> >>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> >>> +					 PWM_COMMIT_MASK, 0x1);
> >>> +		mtk_disp_pwm_update_bits(mdp, DISP_PWM_COMMIT,
> >>> +					 PWM_COMMIT_MASK, 0x0);
> >>
> >> should be renamed to MT8173_PWM_COMMIT_MASK, although I'm not really a
> >> friend of mixing up SoC specific defines with a Soc specific
> >> mtk_disp_pwm structure.
> >>
> >> Regards,
> >> Matthias
> >
> > ok, I will modify it as you said. thanks for you suggestion.
> >
> >>
> >>> +	}
> >>>
> >>>    	clk_disable(mdp->clk_mm);
> >>>    	clk_disable(mdp->clk_main);
> >>> @@ -134,7 +151,9 @@ static int mtk_disp_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> >>>    		return err;
> >>>    	}
> >>>
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 1);
> >>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> >>> +				 mdp->data->enable_bit,
> >>> +				 mdp->data->enable_bit);
> >>>
> >>>    	return 0;
> >>>    }
> >>> @@ -143,7 +162,8 @@ static void mtk_disp_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >>>    {
> >>>    	struct mtk_disp_pwm *mdp = to_mtk_disp_pwm(chip);
> >>>
> >>> -	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN, PWM_ENABLE_MASK, 0);
> >>> +	mtk_disp_pwm_update_bits(mdp, DISP_PWM_EN,
> >>> +				 mdp->data->enable_bit, 0x0);
> >>>
> >>>    	clk_disable(mdp->clk_mm);
> >>>    	clk_disable(mdp->clk_main);
> >>> @@ -156,12 +176,41 @@ static const struct pwm_ops mtk_disp_pwm_ops = {
> >>>    	.owner = THIS_MODULE,
> >>>    };
> >>>
> >>> +static const struct mtk_pwm_data mt8173_pwm_data = {
> >>> +	.enable_bit = BIT(0),
> >>> +	.con0 = 0x10,
> >>> +	.con0_sel = 0x0,
> >>> +	.con1 = 0x14,
> >>> +	.have_commit_reg = true,
> >>> +};
> >>> +
> >>> +static const struct mtk_pwm_data mt2701_pwm_data = {
> >>> +	.enable_bit = BIT(16),
> >>> +	.con0 = 0xa8,
> >>> +	.con0_sel = 0x2,
> >>> +	.con1 = 0xac,
> >>> +	.have_commit_reg = false,
> >>> +};
> >>> +
> >>> +static const struct of_device_id mtk_disp_pwm_of_match[] = {
> >>> +	{ .compatible = "mediatek,mt2701-disp-bls", .data = &mt2701_pwm_data},
> >>> +	{ .compatible = "mediatek,mt6595-disp-pwm", .data = &mt8173_pwm_data},
> >>> +	{ .compatible = "mediatek,mt8173-disp-pwm", .data = &mt8173_pwm_data},
> >>> +	{ }
> >>> +};
> >>> +MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> >>> +
> >>>    static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >>>    {
> >>> +	const struct of_device_id *id;
> >>>    	struct mtk_disp_pwm *mdp;
> >>>    	struct resource *r;
> >>>    	int ret;
> >>>
> >>> +	id = of_match_device(mtk_disp_pwm_of_match, &pdev->dev);
> >>> +	if (!id)
> >>> +		return -EINVAL;
> >>> +
> >>>    	mdp = devm_kzalloc(&pdev->dev, sizeof(*mdp), GFP_KERNEL);
> >>>    	if (!mdp)
> >>>    		return -ENOMEM;
> >>> @@ -191,6 +240,7 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >>>    	mdp->chip.ops = &mtk_disp_pwm_ops;
> >>>    	mdp->chip.base = -1;
> >>>    	mdp->chip.npwm = 1;
> >>> +	mdp->data = id->data;
> >>>
> >>>    	ret = pwmchip_add(&mdp->chip);
> >>>    	if (ret < 0) {
> >>> @@ -200,6 +250,18 @@ static int mtk_disp_pwm_probe(struct platform_device *pdev)
> >>>
> >>>    	platform_set_drvdata(pdev, mdp);
> >>>
> >>> +	/*
> >>> +	 * For MT2701, disable double buffer before writing register
> >>> +	 * and select manual mode and use PWM_PERIOD/PWM_HIGH_WIDTH.
> >>> +	 */
> >>> +	if (!mdp->data->have_commit_reg) {
> >>> +		mtk_disp_pwm_update_bits(mdp, MT2701_PWM_BLS_DEBUG,
> >>> +					 MT2701_PWM_BLS_DEBUG_MASK, 0x3);
> >>> +		mtk_disp_pwm_update_bits(mdp, mdp->data->con0,
> >>> +					 MT2701_PWM_MANUAL_SEL_MASK,
> >>> +					 mdp->data->con0_sel);
> >>> +	}
> >>> +
> >>>    	return 0;
> >>>
> >>>    disable_clk_mm:
> >>> @@ -221,13 +283,6 @@ static int mtk_disp_pwm_remove(struct platform_device *pdev)
> >>>    	return ret;
> >>>    }
> >>>
> >>> -static const struct of_device_id mtk_disp_pwm_of_match[] = {
> >>> -	{ .compatible = "mediatek,mt8173-disp-pwm" },
> >>> -	{ .compatible = "mediatek,mt6595-disp-pwm" },
> >>> -	{ }
> >>> -};
> >>> -MODULE_DEVICE_TABLE(of, mtk_disp_pwm_of_match);
> >>> -
> >>>    static struct platform_driver mtk_disp_pwm_driver = {
> >>>    	.driver = {
> >>>    		.name = "mediatek-disp-pwm",
> >>>
> >
> >

[PATCH 3/3] dts: Add disp bls dsti and dts config for MT2701

[Weiqing Kong]

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 arch/arm/boot/dts/mt2701-evb.dts | 29 +++++++++++++++++++++++++++++
 arch/arm/boot/dts/mt2701.dtsi    |  9 +++++++++
 2 files changed, 38 insertions(+)

diff --git a/arch/arm/boot/dts/mt2701-evb.dts b/arch/arm/boot/dts/mt2701-evb.dts
index 082ca88..6d572f4 100644
--- a/arch/arm/boot/dts/mt2701-evb.dts
+++ b/arch/arm/boot/dts/mt2701-evb.dts
@@ -22,6 +22,35 @@
 	memory {
 		reg = <0 0x80000000 0 0x40000000>;
 	};
+
+	backlight_lcd: backlight_lcd {
+		compatible = "pwm-backlight";
+		pwms = <&disp_bls 0 100000>;
+		brightness-levels = <
+			  0  16  32  48  64  80  96 112
+			128 144 160 176 192 208 224 240
+			255
+		>;
+		default-brightness-level = <9>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pwm_bls_gpio>;
+	};
+};
+
+&disp_bls {
+	status = "okay";
+};
+
+&mmsys {
+	status = "okay";
+};
+
+&pio {
+	pwm_bls_gpio: pwm_bls_gpio {
+		pins_cmd_dat {
+			pinmux = <MT2701_PIN_208_AUD_EXT_CK1__FUNC_DISP_PWM>;
+		};
+	};
 };
 
 &uart0 {
diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
index 42d5a37..dae55d6 100644
--- a/arch/arm/boot/dts/mt2701.dtsi
+++ b/arch/arm/boot/dts/mt2701.dtsi
@@ -234,6 +234,15 @@
 		status = "disabled";
 	};
 
+	disp_bls: dispsys@1400a000 {
+		compatible = "mediatek,mt2701-disp-bls";
+		reg = <0 0x1400a000 0 0x1000>;
+		#pwm-cells = <2>;
+		clocks = <&mmsys CLK_MM_MDP_BLS_26M>, <&mmsys CLK_MM_DISP_BLS>;
+		clock-names = "main", "mm";
+		status = "disabled";
+	};
+
 	imgsys: syscon@15000000 {
 		compatible = "mediatek,mt2701-imgsys", "syscon";
 		reg = <0 0x15000000 0 0x1000>;
-- 
1.8.1.1.dirty

[PATCH 3/3] dts: Add disp bls dsti and dts config for MT2701

[Weiqing Kong]

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 arch/arm/boot/dts/mt2701-evb.dts | 29 +++++++++++++++++++++++++++++
 arch/arm/boot/dts/mt2701.dtsi    |  9 +++++++++
 2 files changed, 38 insertions(+)

diff --git a/arch/arm/boot/dts/mt2701-evb.dts b/arch/arm/boot/dts/mt2701-evb.dts
index 082ca88..6d572f4 100644
--- a/arch/arm/boot/dts/mt2701-evb.dts
+++ b/arch/arm/boot/dts/mt2701-evb.dts
@@ -22,6 +22,35 @@
 	memory {
 		reg = <0 0x80000000 0 0x40000000>;
 	};
+
+	backlight_lcd: backlight_lcd {
+		compatible = "pwm-backlight";
+		pwms = <&disp_bls 0 100000>;
+		brightness-levels = <
+			  0  16  32  48  64  80  96 112
+			128 144 160 176 192 208 224 240
+			255
+		>;
+		default-brightness-level = <9>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pwm_bls_gpio>;
+	};
+};
+
+&disp_bls {
+	status = "okay";
+};
+
+&mmsys {
+	status = "okay";
+};
+
+&pio {
+	pwm_bls_gpio: pwm_bls_gpio {
+		pins_cmd_dat {
+			pinmux = <MT2701_PIN_208_AUD_EXT_CK1__FUNC_DISP_PWM>;
+		};
+	};
 };
 
 &uart0 {
diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
index 42d5a37..dae55d6 100644
--- a/arch/arm/boot/dts/mt2701.dtsi
+++ b/arch/arm/boot/dts/mt2701.dtsi
@@ -234,6 +234,15 @@
 		status = "disabled";
 	};
 
+	disp_bls: dispsys@1400a000 {
+		compatible = "mediatek,mt2701-disp-bls";
+		reg = <0 0x1400a000 0 0x1000>;
+		#pwm-cells = <2>;
+		clocks = <&mmsys CLK_MM_MDP_BLS_26M>, <&mmsys CLK_MM_DISP_BLS>;
+		clock-names = "main", "mm";
+		status = "disabled";
+	};
+
 	imgsys: syscon@15000000 {
 		compatible = "mediatek,mt2701-imgsys", "syscon";
 		reg = <0 0x15000000 0 0x1000>;
-- 
1.8.1.1.dirty

[PATCH 3/3] dts: Add disp bls dsti and dts config for MT2701

[Weiqing Kong]

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 arch/arm/boot/dts/mt2701-evb.dts | 29 +++++++++++++++++++++++++++++
 arch/arm/boot/dts/mt2701.dtsi    |  9 +++++++++
 2 files changed, 38 insertions(+)

diff --git a/arch/arm/boot/dts/mt2701-evb.dts b/arch/arm/boot/dts/mt2701-evb.dts
index 082ca88..6d572f4 100644
--- a/arch/arm/boot/dts/mt2701-evb.dts
+++ b/arch/arm/boot/dts/mt2701-evb.dts
@@ -22,6 +22,35 @@
 	memory {
 		reg = <0 0x80000000 0 0x40000000>;
 	};
+
+	backlight_lcd: backlight_lcd {
+		compatible = "pwm-backlight";
+		pwms = <&disp_bls 0 100000>;
+		brightness-levels = <
+			  0  16  32  48  64  80  96 112
+			128 144 160 176 192 208 224 240
+			255
+		>;
+		default-brightness-level = <9>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pwm_bls_gpio>;
+	};
+};
+
+&disp_bls {
+	status = "okay";
+};
+
+&mmsys {
+	status = "okay";
+};
+
+&pio {
+	pwm_bls_gpio: pwm_bls_gpio {
+		pins_cmd_dat {
+			pinmux = <MT2701_PIN_208_AUD_EXT_CK1__FUNC_DISP_PWM>;
+		};
+	};
 };
 
 &uart0 {
diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
index 42d5a37..dae55d6 100644
--- a/arch/arm/boot/dts/mt2701.dtsi
+++ b/arch/arm/boot/dts/mt2701.dtsi
@@ -234,6 +234,15 @@
 		status = "disabled";
 	};
 
+	disp_bls: dispsys at 1400a000 {
+		compatible = "mediatek,mt2701-disp-bls";
+		reg = <0 0x1400a000 0 0x1000>;
+		#pwm-cells = <2>;
+		clocks = <&mmsys CLK_MM_MDP_BLS_26M>, <&mmsys CLK_MM_DISP_BLS>;
+		clock-names = "main", "mm";
+		status = "disabled";
+	};
+
 	imgsys: syscon at 15000000 {
 		compatible = "mediatek,mt2701-imgsys", "syscon";
 		reg = <0 0x15000000 0 0x1000>;
-- 
1.8.1.1.dirty

[PATCH 0/3] Support Mediatek Soc MT2701 disp pwm

[Weiqing Kong]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

These series patches modify pwm-mtk-disp.c
and dts/dtsi config to support MT2701 disp pwm.

Weiqing Kong (3):
  dt-bindings: pwm: Add MediaTek display PWM bindings
  pwm: Add MediaTek MT2701 display PWM driver support
  dts: Add disp bls dsti and dts config for MT2701

 .../devicetree/bindings/pwm/pwm-mtk-disp.txt       |  5 +-
 arch/arm/boot/dts/mt2701-evb.dts                   | 29 +++++++
 arch/arm/boot/dts/mt2701.dtsi                      |  9 +++
 drivers/pwm/pwm-mtk-disp.c                         | 89 +++++++++++++++++-----
 4 files changed, 113 insertions(+), 19 deletions(-)

-- 
1.9.1

[-- Attachment #2: Type: text/html, Size: 1519 bytes --]

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[Weiqing Kong]

[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]

Add MT2701 compatible string.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
index f8f59ba..f2f2fa9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
@@ -1,9 +1,10 @@
 MediaTek display PWM controller
 
 Required properties:
- - compatible: should be "mediatek,<name>-disp-pwm":
-   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
+ - compatible: should be like these:
+   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
    - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
+   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
-- 
1.8.1.1.dirty

[-- Attachment #2: Type: text/html, Size: 2055 bytes --]

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[Weiqing Kong]

[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]

Add MT2701 compatible string.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
index f8f59ba..f2f2fa9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
@@ -1,9 +1,10 @@
 MediaTek display PWM controller
 
 Required properties:
- - compatible: should be "mediatek,<name>-disp-pwm":
-   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
+ - compatible: should be like these:
+   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
    - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
+   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
-- 
1.8.1.1.dirty

[-- Attachment #2: Type: text/html, Size: 2055 bytes --]

[PATCH 0/3] Support Mediatek Soc MT2701 disp pwm

[weiqing.kong]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

These series patches modify pwm-mtk-disp.c
and dts/dtsi config to support MT2701 disp pwm.

Weiqing Kong (3):
  dt-bindings: pwm: Add MediaTek display PWM bindings
  pwm: Add MediaTek MT2701 display PWM driver support
  dts: Add disp bls dsti and dts config for MT2701

 .../devicetree/bindings/pwm/pwm-mtk-disp.txt       |  5 +-
 arch/arm/boot/dts/mt2701-evb.dts                   | 29 +++++++
 arch/arm/boot/dts/mt2701.dtsi                      |  9 +++
 drivers/pwm/pwm-mtk-disp.c                         | 89 +++++++++++++++++-----
 4 files changed, 113 insertions(+), 19 deletions(-)

-- 
1.9.1

[-- Attachment #2: Type: text/html, Size: 1519 bytes --]

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[weiqing.kong]

[-- Attachment #1: Type: text/plain, Size: 1129 bytes --]

From: Weiqing Kong <weiqing.kong@mediatek.com>

Add MT2701 compatible string.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 .../devicetree/bindings/pwm/pwm-mtk-disp.txt       |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
index f8f59ba..f2f2fa9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
@@ -1,9 +1,10 @@
 MediaTek display PWM controller
 
 Required properties:
- - compatible: should be "mediatek,<name>-disp-pwm":
-   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
+ - compatible: should be like these:
+   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
    - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
+   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
-- 
1.7.9.5

[-- Attachment #2: Type: text/html, Size: 2104 bytes --]

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[weiqing.kong]

[-- Attachment #1: Type: text/plain, Size: 1129 bytes --]

From: Weiqing Kong <weiqing.kong@mediatek.com>

Add MT2701 compatible string.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 .../devicetree/bindings/pwm/pwm-mtk-disp.txt       |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
index f8f59ba..f2f2fa9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
@@ -1,9 +1,10 @@
 MediaTek display PWM controller
 
 Required properties:
- - compatible: should be "mediatek,<name>-disp-pwm":
-   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
+ - compatible: should be like these:
+   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
    - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
+   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
-- 
1.7.9.5

[-- Attachment #2: Type: text/html, Size: 2104 bytes --]

[PATCH 0/3] Support Mediatek Soc MT2701 disp pwm

[Weiqing Kong]

These series patches modify pwm-mtk-disp.c
and dts/dtsi config to support MT2701 disp pwm.

Weiqing Kong (3):
  dt-bindings: pwm: Add MediaTek display PWM bindings
  pwm: Add MediaTek MT2701 display PWM driver support
  dts: Add disp bls dsti and dts config for MT2701

 .../devicetree/bindings/pwm/pwm-mtk-disp.txt       |  5 +-
 arch/arm/boot/dts/mt2701-evb.dts                   | 29 +++++++
 arch/arm/boot/dts/mt2701.dtsi                      |  9 +++
 drivers/pwm/pwm-mtk-disp.c                         | 89 +++++++++++++++++-----
 4 files changed, 113 insertions(+), 19 deletions(-)

-- 
1.9.1

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[Weiqing Kong]

Add MT2701 compatible string.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
index f8f59ba..f2f2fa9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
@@ -1,9 +1,10 @@
 MediaTek display PWM controller
 
 Required properties:
- - compatible: should be "mediatek,<name>-disp-pwm":
-   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
+ - compatible: should be like these:
+   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
    - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
+   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
-- 
1.9.1

[PATCH 0/3] Support Mediatek Soc MT2701 disp pwm

[Weiqing Kong]

These series patches modify pwm-mtk-disp.c
and dts/dtsi config to support MT2701 disp pwm.

Weiqing Kong (3):
  dt-bindings: pwm: Add MediaTek display PWM bindings
  pwm: Add MediaTek MT2701 display PWM driver support
  dts: Add disp bls dsti and dts config for MT2701

 .../devicetree/bindings/pwm/pwm-mtk-disp.txt       |  5 +-
 arch/arm/boot/dts/mt2701-evb.dts                   | 29 +++++++
 arch/arm/boot/dts/mt2701.dtsi                      |  9 +++
 drivers/pwm/pwm-mtk-disp.c                         | 89 +++++++++++++++++-----
 4 files changed, 113 insertions(+), 19 deletions(-)

-- 
1.9.1

[PATCH 1/3] dt-bindings: pwm: Add MediaTek display PWM bindings

[Weiqing Kong]

Add MT2701 compatible string.

Signed-off-by: Weiqing Kong <weiqing.kong@mediatek.com>
---
 Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
index f8f59ba..f2f2fa9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-mtk-disp.txt
@@ -1,9 +1,10 @@
 MediaTek display PWM controller
 
 Required properties:
- - compatible: should be "mediatek,<name>-disp-pwm":
-   - "mediatek,mt8173-disp-pwm": found on mt8173 SoC.
+ - compatible: should be like these:
+   - "mediatek,mt2701-disp-bls": found on mt2701 SoC.
    - "mediatek,mt6595-disp-pwm": found on mt6595 SoC.
+   - "mediatek,mt8173-disp-pwm": found on mt8173 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.
-- 
1.9.1