All of lore.kernel.org
 help / color / mirror / Atom feed
From: Valentin Longchamp <valentin.longchamp@keymile.com>
To: Scott Wood <scottwood@freescale.com>, Wolfram Sang <wsa@the-dreams.de>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	danielle.costantino@gmail.com, "Boschung,
	Rainer" <Rainer.Boschung@keymile.com>,
	"Brunck, Holger" <Holger.Brunck@keymile.com>,
	Linux I2C <linux-i2c@vger.kernel.org>,
	Linux PowerPC Kernel <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH] i2c-qoriq: modified compatibility for correct prescaler
Date: Thu, 11 Dec 2014 14:44:20 +0100	[thread overview]
Message-ID: <54899FB4.9010207@keymile.com> (raw)
In-Reply-To: <1416966097.15957.171.camel@freescale.com>

Hi all,

Picking up this issue again.

On 11/26/2014 02:41 AM, Scott Wood wrote:
> On Tue, 2014-11-25 at 19:13 +0100, Wolfram Sang wrote:
>> On Mon, Nov 17, 2014 at 07:28:03PM -0600, Scott Wood wrote:
>>> On Fri, 2014-11-14 at 09:28 +0100, Wolfram Sang wrote:
>>>>>>
>>>>>> If we're going to change the device tree I'd rather just add a property
>>>>>> to say what the prescaler is.
>>>>>
>>>>>  We would however, leave the boards' device trees that use things like
>>>>> "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it.
>>>>>
>>>>>
>>>>> Now the drawback is that the driver would require a change, to parse this
>>>>> prescaler new prescaler property. Would this be OK from your point of view
>>>>> Wolfram ? If yes, I will send the patches for it.
>>>>
>>>> I don't think it is OK.
>>>
>>> Why?
>>
>> Because I thought it could be deduced. Then, a seperate property would
>> not be OK.
>>
>>>>  I'd think it can be deduced from the compatible property.
>>>
>>> For almost all existing device trees it cannot be.
>>
>> Pity :( If we do introduce a new property, it should probably be
>> "clock-div". Grepping through binding documentation, that seems
>> accepted. We should ask DT maintainers, too, to be safe.
>>
>>> If you want something that will work without changing device trees,
>>> you'll need to use SVR to identify the SoC.
>>
>> The driver is doing that already, see mpc_i2c_get_sec_cfg_8xxx(). Dunno
>> if it makes sense to add to it for consistency reasons?
> 
> That's not SVR, but sure.  Better to avoid messing with existing device
> trees.
> 

What is then the agreement here ? Add a clock-div to the device trees ? Or do
something similar to  mpc_i2c_get_sec_cfg_8xxx() ?

I think the clock-div property is better according to Freescale's AN 2919
section 3.1 Source clock. All the source clocks are fixed (with a clock-div of 2
in case of mpc8536/43/45/47/48/67/68/72, plus p2020) except for the mpc8533/44
where it can be 2 or 3, and that's what mpc_i2c_get_sec_cfg_8xxx() determines.

So mpc_i2c_get_sec_cfg_8xxx() should remain the exception and the other
prescaler values should be derived from an additional clock-div that must be
added in the respective device trees (at least for the qoriq devices, because
for instance mpc8543 already has the correct prescaler thanks to
mpc_i2c_data_8543 from i2c-mpc.c).

Valentin
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

WARNING: multiple messages have this Message-ID (diff)
From: Valentin Longchamp <valentin.longchamp@keymile.com>
To: Scott Wood <scottwood@freescale.com>, Wolfram Sang <wsa@the-dreams.de>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	danielle.costantino@gmail.com, "Boschung,
	Rainer" <Rainer.Boschung@keymile.com>,
	"Brunck, Holger" <Holger.Brunck@keymile.com>,
	Linux I2C <linux-i2c@vger.kernel.org>,
	Linux PowerPC Kernel <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH] i2c-qoriq: modified compatibility for correct prescaler
Date: Thu, 11 Dec 2014 14:44:20 +0100	[thread overview]
Message-ID: <54899FB4.9010207@keymile.com> (raw)
In-Reply-To: <1416966097.15957.171.camel@freescale.com>

Hi all,

Picking up this issue again.

On 11/26/2014 02:41 AM, Scott Wood wrote:
> On Tue, 2014-11-25 at 19:13 +0100, Wolfram Sang wrote:
>> On Mon, Nov 17, 2014 at 07:28:03PM -0600, Scott Wood wrote:
>>> On Fri, 2014-11-14 at 09:28 +0100, Wolfram Sang wrote:
>>>>>>
>>>>>> If we're going to change the device tree I'd rather just add a property
>>>>>> to say what the prescaler is.
>>>>>
>>>>>  We would however, leave the boards' device trees that use things like
>>>>> "fsl,mpc8543-i2c" as is and introduce the prescaler for the others requiring it.
>>>>>
>>>>>
>>>>> Now the drawback is that the driver would require a change, to parse this
>>>>> prescaler new prescaler property. Would this be OK from your point of view
>>>>> Wolfram ? If yes, I will send the patches for it.
>>>>
>>>> I don't think it is OK.
>>>
>>> Why?
>>
>> Because I thought it could be deduced. Then, a seperate property would
>> not be OK.
>>
>>>>  I'd think it can be deduced from the compatible property.
>>>
>>> For almost all existing device trees it cannot be.
>>
>> Pity :( If we do introduce a new property, it should probably be
>> "clock-div". Grepping through binding documentation, that seems
>> accepted. We should ask DT maintainers, too, to be safe.
>>
>>> If you want something that will work without changing device trees,
>>> you'll need to use SVR to identify the SoC.
>>
>> The driver is doing that already, see mpc_i2c_get_sec_cfg_8xxx(). Dunno
>> if it makes sense to add to it for consistency reasons?
> 
> That's not SVR, but sure.  Better to avoid messing with existing device
> trees.
> 

What is then the agreement here ? Add a clock-div to the device trees ? Or do
something similar to  mpc_i2c_get_sec_cfg_8xxx() ?

I think the clock-div property is better according to Freescale's AN 2919
section 3.1 Source clock. All the source clocks are fixed (with a clock-div of 2
in case of mpc8536/43/45/47/48/67/68/72, plus p2020) except for the mpc8533/44
where it can be 2 or 3, and that's what mpc_i2c_get_sec_cfg_8xxx() determines.

So mpc_i2c_get_sec_cfg_8xxx() should remain the exception and the other
prescaler values should be derived from an additional clock-div that must be
added in the respective device trees (at least for the qoriq devices, because
for instance mpc8543 already has the correct prescaler thanks to
mpc_i2c_data_8543 from i2c-mpc.c).

Valentin

  parent reply	other threads:[~2014-12-11 13:44 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-17  9:27 [PATCH] i2c-qoriq: modified compatibility for correct prescaler Valentin Longchamp
2014-10-17  9:27 ` Valentin Longchamp
     [not found] ` <1413538026-15739-1-git-send-email-valentin.longchamp-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
2014-10-28 23:08   ` Scott Wood
2014-10-28 23:08     ` Scott Wood
2014-10-29  8:59     ` Valentin Longchamp
2014-10-29  8:59       ` Valentin Longchamp
     [not found]       ` <5450AC85.40302-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
2014-11-06 21:58         ` Scott Wood
2014-11-06 21:58           ` Scott Wood
2014-11-13  0:34         ` Wolfram Sang
2014-11-13  0:34           ` Wolfram Sang
2014-11-14  7:43           ` Valentin Longchamp
2014-11-14  7:43             ` Valentin Longchamp
     [not found]             ` <5465B285.7070005-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
2014-11-14  8:28               ` Wolfram Sang
2014-11-14  8:28                 ` Wolfram Sang
2014-11-18  1:28                 ` Scott Wood
2014-11-18  1:28                   ` Scott Wood
     [not found]                   ` <1416274083.15957.96.camel-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
2014-11-25 18:13                     ` Wolfram Sang
2014-11-25 18:13                       ` Wolfram Sang
2014-11-26  1:41                       ` Scott Wood
2014-11-26  1:41                         ` Scott Wood
     [not found]                         ` <1416966097.15957.171.camel-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
2014-11-30  4:30                           ` Danielle Costantino
2014-11-30  4:30                             ` Danielle Costantino
     [not found]                             ` <CAAVjN7fJyOh64p5UGRmv-UTnF47cERDfbHyHTuzQmQj8xTDEOQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-12-01 17:23                               ` Wolfram Sang
2014-12-01 17:23                                 ` Wolfram Sang
2014-12-11 13:44                         ` Valentin Longchamp [this message]
2014-12-11 13:44                           ` Valentin Longchamp
     [not found]                           ` <54899FB4.9010207-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
2014-12-23 13:23                             ` Valentin Longchamp
2014-12-23 13:23                               ` Valentin Longchamp
     [not found]                               ` <54996CB5.8030808-SkAbAL50j+5BDgjK7y7TUQ@public.gmane.org>
2014-12-23 13:49                                 ` Wolfram Sang
2014-12-23 13:49                                   ` Wolfram Sang
2014-12-27  2:43                                   ` Scott Wood
2014-12-27  2:43                                     ` Scott Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54899FB4.9010207@keymile.com \
    --to=valentin.longchamp@keymile.com \
    --cc=Holger.Brunck@keymile.com \
    --cc=Rainer.Boschung@keymile.com \
    --cc=danielle.costantino@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=scottwood@freescale.com \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.