From: grygorii.strashko@ti•com (Grygorii Strashko)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes
Date: Thu, 14 Aug 2014 19:54:09 +0300 [thread overview]
Message-ID: <53ECE9B1.4060303@ti.com> (raw)
In-Reply-To: <1408029982.880621027@f126.i.mail.ru>
On 08/14/2014 06:26 PM, Alexander Shiyan wrote:
> Thu, 14 Aug 2014 18:57:08 +0300 ?? Grygorii Strashko <grygorii.strashko@ti•com>:
> ...
>>>>>> + dspgpio7: keystone_dsp_gpio at 262025C {
>>>>>> + compatible = "ti,keystone-mctrl-gpio";
>>>>>> + gpio-controller;
>>>>>> + #gpio-cells = <2>;
>>>>>> + gpio,syscon-dev = <&devctrl 0x25c>;
>>>>>> + };
>>>>>
>>>>> So, devctrl is a syscon device and this DTS introduce several
>>>>> identical GPIO descriptions?
>>>>>
>>>>> On my opinion this should be placed in the gpio-syscon.c,
>>>>> where you can add support for ti,keystone-dsp0{..7}-gpio.
>>>>> Such change will avoid parts 2 and 3 of this patch.
>>>>>
>>>>> static const struct syscon_gpio_data ti_keystone_dsp0_gpio = {
>>>>> .compatible = "ti,keystone-syscon",
>>>>> .dat_bit_offset = 0x240 * 8,
>>>>> ...
>>>>> .set = etc...
>>>>> };
>>>>
>>>> So, if I understand you right, you propose to add 8 additional compatible
>>>> strings just to encode different register offsets. Is it?
>>>> 1) add "ti,keystone-mctrl-gpio0"..."ti,keystone-mctrl-gpio7"
>>>
>>> Yes, but replace "mctrl" with "dsp" since mctrl is not applicable here.
>>>
>>>> 2) add 8 structures keystone_mctrl_gpio0..keystone_mctrl_gpio7 in gpio-syscon.c
>>>> (which will have only different values of field - .dat_bit_offset = 0x2yy * 8,)
>>>>
>>>> 3) add 8 additional items in array syscon_gpio_ids
>>>> {
>>>> .compatible = "ti,keystone-mctrl-gpio0",
>>>> .data = &keystone_mctrl_gpio0,
>>>> }, ...
>>>>
>>>> I can do it if you strictly insist, BUT I don't like it :(
>>>> - just imagine how your driver will look look like if 5 or 6 SoCs will re-use it ;)
>>>> - as I mentioned in cover letter and commit messages even each SoC revision can have
>>>> different Syscon implementation with different registers offsets and with different
>>>> number of Syscon register ranges (for example for Keystone 2 we already have two
>>>> Syscon devices defined in DT).
>>>
>>> The initial version of this driver contains addresses and offsets in, but this approach has
>>> been criticized by DT maintainers.
>>
>> Could you provide link on this thread^, pls?
>
> Here is a link to first version:
> https://www.mail-archive.com/linux-gpio at vger.kernel.org/msg01616.html
10x
>
> Unfortunately, I lost thread for DT-related stuffs.
>
Oh, I've just checked ARM dts directory and found that
constructions like this:
vendor,syscon = <&syscon_phandle>; /// or even this vendor,syscon = <&syscon_phandle 0x60 2>;
are widely used as of now.
Also, According to the bindings doc, the Syscon is not a Linux specific definition (mfd/syscon.txt).
Regards,
-grygorii
next prev parent reply other threads:[~2014-08-14 16:54 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-13 16:16 [PATCH 0/4] gpio: syscon: reuse for keystone 2 socs Grygorii Strashko
2014-08-13 16:16 ` [PATCH 1/4] gpio: syscon: add soc specific callback to assign output value Grygorii Strashko
2014-08-29 6:19 ` Linus Walleij
2014-09-01 14:55 ` Grygorii Strashko
2014-08-13 16:16 ` [PATCH 2/4] gpio: syscon: retrive syscon node and regs offsets from dt Grygorii Strashko
2014-08-13 16:16 ` [PATCH 3/4] gpio: syscon: reuse for keystone 2 socs Grygorii Strashko
2014-08-29 5:53 ` Linus Walleij
2014-08-13 16:16 ` [PATCH 4/4] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes Grygorii Strashko
2014-08-13 16:06 ` Alexander Shiyan
2014-08-14 12:13 ` Grygorii Strashko
2014-08-14 12:12 ` Alexander Shiyan
2014-08-14 15:57 ` Grygorii Strashko
2014-08-14 15:26 ` Alexander Shiyan
2014-08-14 16:54 ` Grygorii Strashko [this message]
2014-08-21 16:23 ` [PATCH v2 0/3] gpio: syscon: reuse for keystone 2 socs Grygorii Strashko
2014-08-21 16:23 ` [PATCH v2 1/3] gpio: syscon: add soc specific callback to assign output value Grygorii Strashko
2014-08-21 16:23 ` [PATCH v2 2/3] gpio: syscon: reuse for keystone 2 socs Grygorii Strashko
2014-08-21 16:23 ` [PATCH v2 3/3] ARM: dts: keystone-k2hk: add dsp gpio controllers nodes Grygorii Strashko
2014-08-21 16:47 ` Alexander Shiyan
2014-08-21 16:51 ` [PATCH v2 0/3] gpio: syscon: reuse for keystone 2 socs Alexander Shiyan
2014-08-28 17:32 ` Grygorii Strashko
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=53ECE9B1.4060303@ti.com \
--to=grygorii.strashko@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox