diff mbox

[RESEND,v3,1/2] dt-bindings: i2c: Add Mediatek MT2701 i2c binding

Message ID 1489029185-21447-2-git-send-email-jun.gao@mediatek.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jun Gao March 9, 2017, 3:13 a.m. UTC
From: Jun Gao <jun.gao@mediatek.com>

Add MT2701 i2c binding to i2c-mt6577.txt and there is no need to
modify i2c driver.

Signed-off-by: Jun Gao <jun.gao@mediatek.com>
---
 .../devicetree/bindings/i2c/i2c-mt6577.txt         |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Rob Herring March 15, 2017, 8:26 p.m. UTC | #1
On Thu, Mar 09, 2017 at 11:13:04AM +0800, Jun Gao wrote:
> From: Jun Gao <jun.gao@mediatek.com>
> 
> Add MT2701 i2c binding to i2c-mt6577.txt and there is no need to
> modify i2c driver.
> 
> Signed-off-by: Jun Gao <jun.gao@mediatek.com>
> ---
>  .../devicetree/bindings/i2c/i2c-mt6577.txt         |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>
Wolfram Sang March 22, 2017, 9:05 a.m. UTC | #2
On Thu, Mar 09, 2017 at 11:13:04AM +0800, Jun Gao wrote:
> From: Jun Gao <jun.gao@mediatek.com>
> 
> Add MT2701 i2c binding to i2c-mt6577.txt and there is no need to
> modify i2c driver.
> 
> Signed-off-by: Jun Gao <jun.gao@mediatek.com>

There is still the issue which Matthias Brugger pointed out on January,
24th: you need to describe the fallback compatibles.

> ---
>  .../devicetree/bindings/i2c/i2c-mt6577.txt         |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> index 0ce6fa3..27dbbf9 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> @@ -4,11 +4,12 @@ The Mediatek's I2C controller is used to interface with I2C devices.
>  
>  Required properties:
>    - compatible: value should be either of the following.
> -      (a) "mediatek,mt6577-i2c", for i2c compatible with mt6577 i2c.
> -      (b) "mediatek,mt6589-i2c", for i2c compatible with mt6589 i2c.
> -      (c) "mediatek,mt8127-i2c", for i2c compatible with mt8127 i2c.
> -      (d) "mediatek,mt8135-i2c", for i2c compatible with mt8135 i2c.
> -      (e) "mediatek,mt8173-i2c", for i2c compatible with mt8173 i2c.
> +	"mediatek,mt2701-i2c"
> +	"mediatek,mt6577-i2c"
> +	"mediatek,mt6589-i2c"
> +	"mediatek,mt8127-i2c"
> +	"mediatek,mt8135-i2c"
> +	"mediatek,mt8173-i2c"
>    - reg: physical base address of the controller and dma base, length of memory
>      mapped region.
>    - interrupts: interrupt number to the cpu.
> -- 
> 1.7.9.5
>
Jun Gao March 28, 2017, 9:50 a.m. UTC | #3
On Wed, 2017-03-22 at 10:05 +0100, Wolfram Sang wrote:
> On Thu, Mar 09, 2017 at 11:13:04AM +0800, Jun Gao wrote:
> > From: Jun Gao <jun.gao@mediatek.com>
> > 
> > Add MT2701 i2c binding to i2c-mt6577.txt and there is no need to
> > modify i2c driver.
> > 
> > Signed-off-by: Jun Gao <jun.gao@mediatek.com>
> 
> There is still the issue which Matthias Brugger pointed out on January,
> 24th: you need to describe the fallback compatibles.
> 

IMHO the value under compatible are just inputs which can be accepted.
It's little strange to add fallback information in binding. Some other
bindings do not describe so detailedly.
Is it OK to make binding as minimum standard?
If we describe it very detailedly, we will have to modify binding if
there are some changes for mt2701.

Rob, could you give some suggestions?  Thanks!

> > ---
> >  .../devicetree/bindings/i2c/i2c-mt6577.txt         |   11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> > index 0ce6fa3..27dbbf9 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> > @@ -4,11 +4,12 @@ The Mediatek's I2C controller is used to interface with I2C devices.
> >  
> >  Required properties:
> >    - compatible: value should be either of the following.
> > -      (a) "mediatek,mt6577-i2c", for i2c compatible with mt6577 i2c.
> > -      (b) "mediatek,mt6589-i2c", for i2c compatible with mt6589 i2c.
> > -      (c) "mediatek,mt8127-i2c", for i2c compatible with mt8127 i2c.
> > -      (d) "mediatek,mt8135-i2c", for i2c compatible with mt8135 i2c.
> > -      (e) "mediatek,mt8173-i2c", for i2c compatible with mt8173 i2c.
> > +	"mediatek,mt2701-i2c"
> > +	"mediatek,mt6577-i2c"
> > +	"mediatek,mt6589-i2c"
> > +	"mediatek,mt8127-i2c"
> > +	"mediatek,mt8135-i2c"
> > +	"mediatek,mt8173-i2c"
> >    - reg: physical base address of the controller and dma base, length of memory
> >      mapped region.
> >    - interrupts: interrupt number to the cpu.
> > -- 
> > 1.7.9.5
> >
Wolfram Sang March 28, 2017, 7:22 p.m. UTC | #4
On Tue, Mar 28, 2017 at 05:50:12PM +0800, Jun Gao wrote:
> On Wed, 2017-03-22 at 10:05 +0100, Wolfram Sang wrote:
> > On Thu, Mar 09, 2017 at 11:13:04AM +0800, Jun Gao wrote:
> > > From: Jun Gao <jun.gao@mediatek.com>
> > > 
> > > Add MT2701 i2c binding to i2c-mt6577.txt and there is no need to
> > > modify i2c driver.
> > > 
> > > Signed-off-by: Jun Gao <jun.gao@mediatek.com>
> > 
> > There is still the issue which Matthias Brugger pointed out on January,
> > 24th: you need to describe the fallback compatibles.
> > 
> 
> IMHO the value under compatible are just inputs which can be accepted.
> It's little strange to add fallback information in binding. Some other
> bindings do not describe so detailedly.
> Is it OK to make binding as minimum standard?
> If we describe it very detailedly, we will have to modify binding if
> there are some changes for mt2701.

My reading of the below is that I could simply use "mediatek,mt2701-i2c"
as compatible and things will work. But it won't, we don't have that in
the driver IIRC. So, we need a fallback for that to work.

> Rob, could you give some suggestions?  Thanks!

Would be welcome, yes. I lost track what the preferred solution is.

> 
> > > ---
> > >  .../devicetree/bindings/i2c/i2c-mt6577.txt         |   11 ++++++-----
> > >  1 file changed, 6 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> > > index 0ce6fa3..27dbbf9 100644
> > > --- a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> > > +++ b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> > > @@ -4,11 +4,12 @@ The Mediatek's I2C controller is used to interface with I2C devices.
> > >  
> > >  Required properties:
> > >    - compatible: value should be either of the following.
> > > -      (a) "mediatek,mt6577-i2c", for i2c compatible with mt6577 i2c.
> > > -      (b) "mediatek,mt6589-i2c", for i2c compatible with mt6589 i2c.
> > > -      (c) "mediatek,mt8127-i2c", for i2c compatible with mt8127 i2c.
> > > -      (d) "mediatek,mt8135-i2c", for i2c compatible with mt8135 i2c.
> > > -      (e) "mediatek,mt8173-i2c", for i2c compatible with mt8173 i2c.
> > > +	"mediatek,mt2701-i2c"
> > > +	"mediatek,mt6577-i2c"
> > > +	"mediatek,mt6589-i2c"
> > > +	"mediatek,mt8127-i2c"
> > > +	"mediatek,mt8135-i2c"
> > > +	"mediatek,mt8173-i2c"
> > >    - reg: physical base address of the controller and dma base, length of memory
> > >      mapped region.
> > >    - interrupts: interrupt number to the cpu.
> > > -- 
> > > 1.7.9.5
> > > 
> 
>
Matthias Brugger May 10, 2017, 10:35 a.m. UTC | #5
On 28/03/17 21:22, Wolfram Sang wrote:
> On Tue, Mar 28, 2017 at 05:50:12PM +0800, Jun Gao wrote:
>> On Wed, 2017-03-22 at 10:05 +0100, Wolfram Sang wrote:
>>> On Thu, Mar 09, 2017 at 11:13:04AM +0800, Jun Gao wrote:
>>>> From: Jun Gao <jun.gao@mediatek.com>
>>>>
>>>> Add MT2701 i2c binding to i2c-mt6577.txt and there is no need to
>>>> modify i2c driver.
>>>>
>>>> Signed-off-by: Jun Gao <jun.gao@mediatek.com>
>>>
>>> There is still the issue which Matthias Brugger pointed out on January,
>>> 24th: you need to describe the fallback compatibles.
>>>
>>
>> IMHO the value under compatible are just inputs which can be accepted.
>> It's little strange to add fallback information in binding. Some other
>> bindings do not describe so detailedly.
>> Is it OK to make binding as minimum standard?
>> If we describe it very detailedly, we will have to modify binding if
>> there are some changes for mt2701.
> 
> My reading of the below is that I could simply use "mediatek,mt2701-i2c"
> as compatible and things will work. But it won't, we don't have that in
> the driver IIRC. So, we need a fallback for that to work.
> 
>> Rob, could you give some suggestions?  Thanks!
> 
> Would be welcome, yes. I lost track what the preferred solution is.
> 

We will need to define the fallback binding for each SoC.
As example take the rockchip mmc:
http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt

Regards,
Matthias

>>
>>>> ---
>>>>   .../devicetree/bindings/i2c/i2c-mt6577.txt         |   11 ++++++-----
>>>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
>>>> index 0ce6fa3..27dbbf9 100644
>>>> --- a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
>>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
>>>> @@ -4,11 +4,12 @@ The Mediatek's I2C controller is used to interface with I2C devices.
>>>>   
>>>>   Required properties:
>>>>     - compatible: value should be either of the following.
>>>> -      (a) "mediatek,mt6577-i2c", for i2c compatible with mt6577 i2c.
>>>> -      (b) "mediatek,mt6589-i2c", for i2c compatible with mt6589 i2c.
>>>> -      (c) "mediatek,mt8127-i2c", for i2c compatible with mt8127 i2c.
>>>> -      (d) "mediatek,mt8135-i2c", for i2c compatible with mt8135 i2c.
>>>> -      (e) "mediatek,mt8173-i2c", for i2c compatible with mt8173 i2c.
>>>> +	"mediatek,mt2701-i2c"
>>>> +	"mediatek,mt6577-i2c"
>>>> +	"mediatek,mt6589-i2c"
>>>> +	"mediatek,mt8127-i2c"
>>>> +	"mediatek,mt8135-i2c"
>>>> +	"mediatek,mt8173-i2c"
>>>>     - reg: physical base address of the controller and dma base, length of memory
>>>>       mapped region.
>>>>     - interrupts: interrupt number to the cpu.
>>>> -- 
>>>> 1.7.9.5
>>>>
>>
>>
Jun Gao May 23, 2017, 3:34 a.m. UTC | #6
On Wed, 2017-05-10 at 12:35 +0200, Matthias Brugger wrote:
> 
> On 28/03/17 21:22, Wolfram Sang wrote:
> > On Tue, Mar 28, 2017 at 05:50:12PM +0800, Jun Gao wrote:
> >> On Wed, 2017-03-22 at 10:05 +0100, Wolfram Sang wrote:
> >>> On Thu, Mar 09, 2017 at 11:13:04AM +0800, Jun Gao wrote:
> >>>> From: Jun Gao <jun.gao@mediatek.com>
> >>>>
> >>>> Add MT2701 i2c binding to i2c-mt6577.txt and there is no need to
> >>>> modify i2c driver.
> >>>>
> >>>> Signed-off-by: Jun Gao <jun.gao@mediatek.com>
> >>>
> >>> There is still the issue which Matthias Brugger pointed out on January,
> >>> 24th: you need to describe the fallback compatibles.
> >>>
> >>
> >> IMHO the value under compatible are just inputs which can be accepted.
> >> It's little strange to add fallback information in binding. Some other
> >> bindings do not describe so detailedly.
> >> Is it OK to make binding as minimum standard?
> >> If we describe it very detailedly, we will have to modify binding if
> >> there are some changes for mt2701.
> > 
> > My reading of the below is that I could simply use "mediatek,mt2701-i2c"
> > as compatible and things will work. But it won't, we don't have that in
> > the driver IIRC. So, we need a fallback for that to work.
> > 
> >> Rob, could you give some suggestions?  Thanks!
> > 
> > Would be welcome, yes. I lost track what the preferred solution is.
> > 
> 
> We will need to define the fallback binding for each SoC.
> As example take the rockchip mmc:
> http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
> 
> Regards,
> Matthias
> 

Hi Rob,

There is no such requirement for other components.
Could you give us some suggestions? 
Thanks!


> >>
> >>>> ---
> >>>>   .../devicetree/bindings/i2c/i2c-mt6577.txt         |   11 ++++++-----
> >>>>   1 file changed, 6 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> >>>> index 0ce6fa3..27dbbf9 100644
> >>>> --- a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> >>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> >>>> @@ -4,11 +4,12 @@ The Mediatek's I2C controller is used to interface with I2C devices.
> >>>>   
> >>>>   Required properties:
> >>>>     - compatible: value should be either of the following.
> >>>> -      (a) "mediatek,mt6577-i2c", for i2c compatible with mt6577 i2c.
> >>>> -      (b) "mediatek,mt6589-i2c", for i2c compatible with mt6589 i2c.
> >>>> -      (c) "mediatek,mt8127-i2c", for i2c compatible with mt8127 i2c.
> >>>> -      (d) "mediatek,mt8135-i2c", for i2c compatible with mt8135 i2c.
> >>>> -      (e) "mediatek,mt8173-i2c", for i2c compatible with mt8173 i2c.
> >>>> +	"mediatek,mt2701-i2c"
> >>>> +	"mediatek,mt6577-i2c"
> >>>> +	"mediatek,mt6589-i2c"
> >>>> +	"mediatek,mt8127-i2c"
> >>>> +	"mediatek,mt8135-i2c"
> >>>> +	"mediatek,mt8173-i2c"
> >>>>     - reg: physical base address of the controller and dma base, length of memory
> >>>>       mapped region.
> >>>>     - interrupts: interrupt number to the cpu.
> >>>> -- 
> >>>> 1.7.9.5
> >>>>
> >>
> >>
Matthias Brugger May 23, 2017, 6:31 a.m. UTC | #7
On 23/05/17 05:34, Jun Gao wrote:
> On Wed, 2017-05-10 at 12:35 +0200, Matthias Brugger wrote:
>>
>> On 28/03/17 21:22, Wolfram Sang wrote:
>>> On Tue, Mar 28, 2017 at 05:50:12PM +0800, Jun Gao wrote:
>>>> On Wed, 2017-03-22 at 10:05 +0100, Wolfram Sang wrote:
>>>>> On Thu, Mar 09, 2017 at 11:13:04AM +0800, Jun Gao wrote:
>>>>>> From: Jun Gao <jun.gao@mediatek.com>
>>>>>>
>>>>>> Add MT2701 i2c binding to i2c-mt6577.txt and there is no need to
>>>>>> modify i2c driver.
>>>>>>
>>>>>> Signed-off-by: Jun Gao <jun.gao@mediatek.com>
>>>>>
>>>>> There is still the issue which Matthias Brugger pointed out on January,
>>>>> 24th: you need to describe the fallback compatibles.
>>>>>
>>>>
>>>> IMHO the value under compatible are just inputs which can be accepted.
>>>> It's little strange to add fallback information in binding. Some other
>>>> bindings do not describe so detailedly.
>>>> Is it OK to make binding as minimum standard?
>>>> If we describe it very detailedly, we will have to modify binding if
>>>> there are some changes for mt2701.
>>>
>>> My reading of the below is that I could simply use "mediatek,mt2701-i2c"
>>> as compatible and things will work. But it won't, we don't have that in
>>> the driver IIRC. So, we need a fallback for that to work.
>>>
>>>> Rob, could you give some suggestions?  Thanks!
>>>
>>> Would be welcome, yes. I lost track what the preferred solution is.
>>>
>>
>> We will need to define the fallback binding for each SoC.
>> As example take the rockchip mmc:
>> http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt
>>
>> Regards,
>> Matthias
>>
> 
> Hi Rob,
> 
> There is no such requirement for other components.

The requirement is that the binding description reflects a compatible 
the driver is able to consume. Right now if you pass 
"mediatek,mt2701-i2c" the driver won't recognize this, therefor you need 
the fallback compatible. Which point isn't clear to you?

Regards,
Matthias

> Could you give us some suggestions?
> Thanks!
> 
> 
>>>>
>>>>>> ---
>>>>>>    .../devicetree/bindings/i2c/i2c-mt6577.txt         |   11 ++++++-----
>>>>>>    1 file changed, 6 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
>>>>>> index 0ce6fa3..27dbbf9 100644
>>>>>> --- a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
>>>>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
>>>>>> @@ -4,11 +4,12 @@ The Mediatek's I2C controller is used to interface with I2C devices.
>>>>>>    
>>>>>>    Required properties:
>>>>>>      - compatible: value should be either of the following.
>>>>>> -      (a) "mediatek,mt6577-i2c", for i2c compatible with mt6577 i2c.
>>>>>> -      (b) "mediatek,mt6589-i2c", for i2c compatible with mt6589 i2c.
>>>>>> -      (c) "mediatek,mt8127-i2c", for i2c compatible with mt8127 i2c.
>>>>>> -      (d) "mediatek,mt8135-i2c", for i2c compatible with mt8135 i2c.
>>>>>> -      (e) "mediatek,mt8173-i2c", for i2c compatible with mt8173 i2c.
>>>>>> +	"mediatek,mt2701-i2c"
>>>>>> +	"mediatek,mt6577-i2c"
>>>>>> +	"mediatek,mt6589-i2c"
>>>>>> +	"mediatek,mt8127-i2c"
>>>>>> +	"mediatek,mt8135-i2c"
>>>>>> +	"mediatek,mt8173-i2c"
>>>>>>      - reg: physical base address of the controller and dma base, length of memory
>>>>>>        mapped region.
>>>>>>      - interrupts: interrupt number to the cpu.
>>>>>> -- 
>>>>>> 1.7.9.5
>>>>>>
>>>>
>>>>
> 
>
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
index 0ce6fa3..27dbbf9 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
@@ -4,11 +4,12 @@  The Mediatek's I2C controller is used to interface with I2C devices.
 
 Required properties:
   - compatible: value should be either of the following.
-      (a) "mediatek,mt6577-i2c", for i2c compatible with mt6577 i2c.
-      (b) "mediatek,mt6589-i2c", for i2c compatible with mt6589 i2c.
-      (c) "mediatek,mt8127-i2c", for i2c compatible with mt8127 i2c.
-      (d) "mediatek,mt8135-i2c", for i2c compatible with mt8135 i2c.
-      (e) "mediatek,mt8173-i2c", for i2c compatible with mt8173 i2c.
+	"mediatek,mt2701-i2c"
+	"mediatek,mt6577-i2c"
+	"mediatek,mt6589-i2c"
+	"mediatek,mt8127-i2c"
+	"mediatek,mt8135-i2c"
+	"mediatek,mt8173-i2c"
   - reg: physical base address of the controller and dma base, length of memory
     mapped region.
   - interrupts: interrupt number to the cpu.