From: emilio@elopez.com.ar (Emilio López)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] memory: add a basic OF-based memory driver
Date: Thu, 12 Sep 2013 22:31:08 -0300 [thread overview]
Message-ID: <52326ADC.8040703@elopez.com.ar> (raw)
In-Reply-To: <CAOesGMiUMniKRLRgGPsimR0bXWQFbx+SL5dUJXOb8t0JfEcHsg@mail.gmail.com>
Hi Olof,
El 12/09/13 21:57, Olof Johansson escribi?:
> On Thu, Sep 12, 2013 at 5:30 PM, Emilio L?pez <emilio@elopez.com.ar> wrote:
>> This driver's only job is to claim and ensure the necessary clock
>> for memory operation on a DT-powered machine remains enabled.
>>
>> Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
>> ---
>>
>> I believe this new patch should resolve all the concerns raised; as
>> always, all feedback is welcome :)
>
> I think you're going about this the wrong way.
>
> If you have a problem with a clock not staying on, shouldn't you just
> marking it appropriately in the clock table instead, making sure it's
> initialized with at least one reference to it?
If by "the clock table" you mean the tree as handled by the common clock
framework, there is no such flag available as of today; see Mike's reply
for more information.
Personally I feel that if the general case can solve our problems (in
this case, having a consumer who prepares and enables the clock), we
should avoid adding special cases to the framework.
> I believe that is how
> some of the other platforms handle this, and it's a lot cleaner than
> adding a fake binding and a fake driver just to grab a single clock.
The binding doesn't have to be fake; it is actually describing the
memory controller hardware:
mc: mc at 0123000 {
compatible = "simple-memory-controller";
reg = <0x0123000 0x400>;
clocks = <&pll5 1>;
};
If one day we get docs and/or have any special features we may need from
the controller, we can use something like
mc: mc at 0123000 {
compatible = "vendor,awesome-mc", "simple-memory-controller";
reg = <0x0123000 0x400>;
clocks = <&pll5 1>;
};
Cheers,
Emilio
WARNING: multiple messages have this Message-ID (diff)
From: "Emilio López" <emilio-0Z03zUJReD5OxF6Tv1QG9Q@public.gmane.org>
To: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>
Cc: "Mike Turquette"
<mturquette-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
"Maxime Ripard"
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
"Grant Likely"
<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
"Rob Herring"
<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>,
"Greg Kroah-Hartman"
<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
"David Lanzendörfer"
<david.lanzendoerfer-Z7Kmv9EsliU@public.gmane.org>,
"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: [PATCH] memory: add a basic OF-based memory driver
Date: Thu, 12 Sep 2013 22:31:08 -0300 [thread overview]
Message-ID: <52326ADC.8040703@elopez.com.ar> (raw)
In-Reply-To: <CAOesGMiUMniKRLRgGPsimR0bXWQFbx+SL5dUJXOb8t0JfEcHsg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Olof,
El 12/09/13 21:57, Olof Johansson escribió:
> On Thu, Sep 12, 2013 at 5:30 PM, Emilio López <emilio-0Z03zUJReD5OxF6Tv1QG9Q@public.gmane.org> wrote:
>> This driver's only job is to claim and ensure the necessary clock
>> for memory operation on a DT-powered machine remains enabled.
>>
>> Signed-off-by: Emilio López <emilio-0Z03zUJReD5OxF6Tv1QG9Q@public.gmane.org>
>> ---
>>
>> I believe this new patch should resolve all the concerns raised; as
>> always, all feedback is welcome :)
>
> I think you're going about this the wrong way.
>
> If you have a problem with a clock not staying on, shouldn't you just
> marking it appropriately in the clock table instead, making sure it's
> initialized with at least one reference to it?
If by "the clock table" you mean the tree as handled by the common clock
framework, there is no such flag available as of today; see Mike's reply
for more information.
Personally I feel that if the general case can solve our problems (in
this case, having a consumer who prepares and enables the clock), we
should avoid adding special cases to the framework.
> I believe that is how
> some of the other platforms handle this, and it's a lot cleaner than
> adding a fake binding and a fake driver just to grab a single clock.
The binding doesn't have to be fake; it is actually describing the
memory controller hardware:
mc: mc@0123000 {
compatible = "simple-memory-controller";
reg = <0x0123000 0x400>;
clocks = <&pll5 1>;
};
If one day we get docs and/or have any special features we may need from
the controller, we can use something like
mc: mc@0123000 {
compatible = "vendor,awesome-mc", "simple-memory-controller";
reg = <0x0123000 0x400>;
clocks = <&pll5 1>;
};
Cheers,
Emilio
--
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
WARNING: multiple messages have this Message-ID (diff)
From: "Emilio López" <emilio@elopez.com.ar>
To: Olof Johansson <olof@lixom.net>
Cc: "Mike Turquette" <mturquette@linaro.org>,
"Maxime Ripard" <maxime.ripard@free-electrons.com>,
"Grant Likely" <grant.likely@linaro.org>,
"Rob Herring" <rob.herring@calxeda.com>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"David Lanzendörfer" <david.lanzendoerfer@o2s.ch>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] memory: add a basic OF-based memory driver
Date: Thu, 12 Sep 2013 22:31:08 -0300 [thread overview]
Message-ID: <52326ADC.8040703@elopez.com.ar> (raw)
In-Reply-To: <CAOesGMiUMniKRLRgGPsimR0bXWQFbx+SL5dUJXOb8t0JfEcHsg@mail.gmail.com>
Hi Olof,
El 12/09/13 21:57, Olof Johansson escribió:
> On Thu, Sep 12, 2013 at 5:30 PM, Emilio López <emilio@elopez.com.ar> wrote:
>> This driver's only job is to claim and ensure the necessary clock
>> for memory operation on a DT-powered machine remains enabled.
>>
>> Signed-off-by: Emilio López <emilio@elopez.com.ar>
>> ---
>>
>> I believe this new patch should resolve all the concerns raised; as
>> always, all feedback is welcome :)
>
> I think you're going about this the wrong way.
>
> If you have a problem with a clock not staying on, shouldn't you just
> marking it appropriately in the clock table instead, making sure it's
> initialized with at least one reference to it?
If by "the clock table" you mean the tree as handled by the common clock
framework, there is no such flag available as of today; see Mike's reply
for more information.
Personally I feel that if the general case can solve our problems (in
this case, having a consumer who prepares and enables the clock), we
should avoid adding special cases to the framework.
> I believe that is how
> some of the other platforms handle this, and it's a lot cleaner than
> adding a fake binding and a fake driver just to grab a single clock.
The binding doesn't have to be fake; it is actually describing the
memory controller hardware:
mc: mc@0123000 {
compatible = "simple-memory-controller";
reg = <0x0123000 0x400>;
clocks = <&pll5 1>;
};
If one day we get docs and/or have any special features we may need from
the controller, we can use something like
mc: mc@0123000 {
compatible = "vendor,awesome-mc", "simple-memory-controller";
reg = <0x0123000 0x400>;
clocks = <&pll5 1>;
};
Cheers,
Emilio
next prev parent reply other threads:[~2013-09-13 1:31 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-11 1:43 [PATCH RFC] of: add a basic memory driver Emilio López
2013-09-11 1:43 ` Emilio López
2013-09-11 7:54 ` Maxime Ripard
2013-09-11 7:54 ` Maxime Ripard
2013-09-11 9:34 ` Emilio López
2013-09-11 9:34 ` Emilio López
2013-09-12 0:21 ` Mike Turquette
2013-09-12 0:21 ` Mike Turquette
2013-09-11 16:40 ` Rob Herring
2013-09-11 16:40 ` Rob Herring
2013-09-13 0:30 ` [PATCH] memory: add a basic OF-based " Emilio López
2013-09-13 0:30 ` Emilio López
2013-09-13 0:30 ` Emilio López
2013-09-13 0:57 ` Olof Johansson
2013-09-13 0:57 ` Olof Johansson
2013-09-13 0:57 ` Olof Johansson
2013-09-13 1:31 ` Emilio López [this message]
2013-09-13 1:31 ` Emilio López
2013-09-13 1:31 ` Emilio López
2013-09-13 14:00 ` Rob Herring
2013-09-13 14:00 ` Rob Herring
2013-09-13 15:49 ` Olof Johansson
2013-09-13 15:49 ` Olof Johansson
2013-09-13 15:49 ` Olof Johansson
2013-09-15 12:43 ` Grant Likely
2013-09-15 12:43 ` Grant Likely
2013-09-16 12:55 ` Rob Herring
2013-09-16 12:55 ` Rob Herring
2013-09-16 12:55 ` Rob Herring
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=52326ADC.8040703@elopez.com.ar \
--to=emilio@elopez.com.ar \
--cc=linux-arm-kernel@lists.infradead.org \
/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.