From: Kevin Hilman <khilman@kernel.org>
To: Eric Anholt <eric@anholt.net>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
linux-arm-kernel@lists.infradead.org,
linux-rpi-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org,
Stephen Warren <swarren@wwwdotorg.org>,
Lee Jones <lee@kernel.org>,
Florian Fainelli <f.fainelli@gmail.com>,
Ulf Hansson <ulf.hansson@linaro.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Alexander Aring <alex.aring@gmail.com>,
devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
Mark Rutland <mark.rutland@arm.com>,
Ian Campbell <ijc+devicetree@hellion.org.uk>
Subject: Re: [PATCH v2 3/5] ARM: bcm2835: add rpi power domain driver
Date: Tue, 08 Dec 2015 10:19:26 -0800 [thread overview]
Message-ID: <7hvb88ls5t.fsf@deeprootsystems.com> (raw)
In-Reply-To: <87fuzdiwcl.fsf@eliezer.anholt.net> (Eric Anholt's message of "Mon, 07 Dec 2015 17:04:58 -0800")
Hi Eric,
Eric Anholt <eric@anholt.net> writes:
> Kevin Hilman <khilman@kernel.org> writes:
>
>> Eric Anholt <eric@anholt.net> writes:
>>
>>> From: Alexander Aring <alex.aring@gmail.com>
>>>
>>> This patch adds support for several power domains on Raspberry Pi,
>>> including USB (so it can be enabled even if the bootloader didn't do
>>> it), and graphics.
>>>
>>> This patch is the combined work of Eric Anholt (who wrote USB support
>>> inside of the Raspberry Pi firmware driver, and wrote the non-USB
>>> domain support) and Alexander Aring (who separated the original USB
>>> work out from the firmware driver).
>>>
>>> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>> ---
>>>
>>> v2: Add support for power domains other than USB, using the new
>>> firmware interface, reword commit message (changes by Eric)
>>
>> [...]
>>
>>> +/*
>>> + * Firmware indices for the old power domains interface. Only a few
>>> + * of them were actually implemented.
>>> + */
>>> +#define RPI_OLD_POWER_DOMAIN_USB 3
>>> +#define RPI_OLD_POWER_DOMAIN_V3D 10
>>> +
>>
>> Is "old" the right word here? Are there firmware versions that could be
>> used instead? What happens when the firwmware is updated next time?
>
> Old is a good word. It's the old interface.
Sure, but "old" is relative and based on experience, folks come to
regret those kinds of names.
> As for what happens when the firmware is updated: Nothing. The firmware
> is updated all the time, and it maintains backwards compatibility.
> Unless you mean "what happens when a newer, fancier power domain
> interface is created and we need a name for the newer one" and the
> answer is "this is a define entirely within the driver, and we can just
> rename it when we want to."
Sure, it's very contained in this driver, so it's ultimately up to you.
It's not something worth blocking this about, I just wanted to be sure
since I'm not very familiar with how the rpi firmware evolves.
>> [...]
>>
>>> + /*
>>> + * Use the old firmware interface for USB power, so that we
>>> + * can turn it on even if the firmware hasn't been updated.
>>> + */
>>> + rpi_init_old_power_domain(rpi_domains, RPI_POWER_DOMAIN_USB,
>>> + RPI_OLD_POWER_DOMAIN_USB, "USB");
>>
>> This seems a bit restrictive.
>>
>> To me, it seems that determining "old" or "new" (or revision of fw
>> interface to use) should be described in DT, not hard-coded in the power
>> domain driver.
>>
>> What about an additional DT property to describe that? or possibly
>> another cell in the domain which could be used to optionally set
>> old/legacy.
>
> As the author and maintainer of the code, I don't feel it's restrictive.
> The firmware protocol is defined and is guaranteed to continue to exist,
> it's only useful for this platform, and defining a new set of custom
> devicetree properties for it would only obfuscate the implementation.
> DT is a useful tool for separating out the between-board differences for
> the same piece of hardware across multiple implementations at different
> addresses, while this is neither hardware nor in multiple
> implementations at different addresses.
That being said, firmware revisions are also very often something that
qualifies as a difference between boards.
Anyways, as I said above, I think this is a potential future problem,
but it's not a big deal to me since it's very self contained.
Kevin
WARNING: multiple messages have this Message-ID (diff)
From: khilman@kernel.org (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 3/5] ARM: bcm2835: add rpi power domain driver
Date: Tue, 08 Dec 2015 10:19:26 -0800 [thread overview]
Message-ID: <7hvb88ls5t.fsf@deeprootsystems.com> (raw)
In-Reply-To: <87fuzdiwcl.fsf@eliezer.anholt.net> (Eric Anholt's message of "Mon, 07 Dec 2015 17:04:58 -0800")
Hi Eric,
Eric Anholt <eric@anholt.net> writes:
> Kevin Hilman <khilman@kernel.org> writes:
>
>> Eric Anholt <eric@anholt.net> writes:
>>
>>> From: Alexander Aring <alex.aring@gmail.com>
>>>
>>> This patch adds support for several power domains on Raspberry Pi,
>>> including USB (so it can be enabled even if the bootloader didn't do
>>> it), and graphics.
>>>
>>> This patch is the combined work of Eric Anholt (who wrote USB support
>>> inside of the Raspberry Pi firmware driver, and wrote the non-USB
>>> domain support) and Alexander Aring (who separated the original USB
>>> work out from the firmware driver).
>>>
>>> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
>>> Signed-off-by: Eric Anholt <eric@anholt.net>
>>> ---
>>>
>>> v2: Add support for power domains other than USB, using the new
>>> firmware interface, reword commit message (changes by Eric)
>>
>> [...]
>>
>>> +/*
>>> + * Firmware indices for the old power domains interface. Only a few
>>> + * of them were actually implemented.
>>> + */
>>> +#define RPI_OLD_POWER_DOMAIN_USB 3
>>> +#define RPI_OLD_POWER_DOMAIN_V3D 10
>>> +
>>
>> Is "old" the right word here? Are there firmware versions that could be
>> used instead? What happens when the firwmware is updated next time?
>
> Old is a good word. It's the old interface.
Sure, but "old" is relative and based on experience, folks come to
regret those kinds of names.
> As for what happens when the firmware is updated: Nothing. The firmware
> is updated all the time, and it maintains backwards compatibility.
> Unless you mean "what happens when a newer, fancier power domain
> interface is created and we need a name for the newer one" and the
> answer is "this is a define entirely within the driver, and we can just
> rename it when we want to."
Sure, it's very contained in this driver, so it's ultimately up to you.
It's not something worth blocking this about, I just wanted to be sure
since I'm not very familiar with how the rpi firmware evolves.
>> [...]
>>
>>> + /*
>>> + * Use the old firmware interface for USB power, so that we
>>> + * can turn it on even if the firmware hasn't been updated.
>>> + */
>>> + rpi_init_old_power_domain(rpi_domains, RPI_POWER_DOMAIN_USB,
>>> + RPI_OLD_POWER_DOMAIN_USB, "USB");
>>
>> This seems a bit restrictive.
>>
>> To me, it seems that determining "old" or "new" (or revision of fw
>> interface to use) should be described in DT, not hard-coded in the power
>> domain driver.
>>
>> What about an additional DT property to describe that? or possibly
>> another cell in the domain which could be used to optionally set
>> old/legacy.
>
> As the author and maintainer of the code, I don't feel it's restrictive.
> The firmware protocol is defined and is guaranteed to continue to exist,
> it's only useful for this platform, and defining a new set of custom
> devicetree properties for it would only obfuscate the implementation.
> DT is a useful tool for separating out the between-board differences for
> the same piece of hardware across multiple implementations at different
> addresses, while this is neither hardware nor in multiple
> implementations at different addresses.
That being said, firmware revisions are also very often something that
qualifies as a difference between boards.
Anyways, as I said above, I think this is a potential future problem,
but it's not a big deal to me since it's very self contained.
Kevin
next prev parent reply other threads:[~2015-12-08 18:19 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-04 17:45 [PATCH v2 0/5] Raspberry Pi power domains v2 Eric Anholt
2015-12-04 17:45 ` Eric Anholt
2015-12-04 17:45 ` Eric Anholt
2015-12-04 17:45 ` [PATCH v2 1/5] power: domain: add pm_genpd_exit Eric Anholt
2015-12-04 17:45 ` Eric Anholt
2015-12-04 17:45 ` Eric Anholt
2015-12-07 10:04 ` Jon Hunter
2015-12-07 10:04 ` Jon Hunter
2015-12-07 10:04 ` Jon Hunter
2015-12-08 18:59 ` Kevin Hilman
2015-12-08 18:59 ` Kevin Hilman
2015-12-08 18:59 ` Kevin Hilman
2015-12-09 10:47 ` Alexander Aring
2015-12-09 10:47 ` Alexander Aring
2015-12-09 10:54 ` Russell King - ARM Linux
2015-12-09 10:54 ` Russell King - ARM Linux
2015-12-14 9:49 ` Ulf Hansson
2015-12-14 9:49 ` Ulf Hansson
2015-12-14 9:49 ` Ulf Hansson
2015-12-04 17:45 ` [PATCH v2 2/5] ARM: bcm2835: Define two new packets from the latest firmware Eric Anholt
2015-12-04 17:45 ` Eric Anholt
2015-12-04 17:45 ` Eric Anholt
2015-12-04 17:45 ` [PATCH v2 3/5] ARM: bcm2835: add rpi power domain driver Eric Anholt
2015-12-04 17:45 ` Eric Anholt
2015-12-07 23:35 ` Kevin Hilman
2015-12-07 23:35 ` Kevin Hilman
2015-12-08 1:04 ` Eric Anholt
2015-12-08 1:04 ` Eric Anholt
2015-12-08 18:19 ` Kevin Hilman [this message]
2015-12-08 18:19 ` Kevin Hilman
2015-12-11 18:13 ` Stefan Wahren
2015-12-11 18:13 ` Stefan Wahren
2015-12-11 18:13 ` Stefan Wahren
2015-12-04 17:45 ` [PATCH v2 4/5] dt-bindings: add rpi power domain driver bindings Eric Anholt
2015-12-04 17:45 ` Eric Anholt
2015-12-04 17:45 ` [PATCH v2 5/5] ARM: bcm2835: Add the Raspberry Pi power domain driver to the DT Eric Anholt
2015-12-04 17:45 ` Eric Anholt
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=7hvb88ls5t.fsf@deeprootsystems.com \
--to=khilman@kernel.org \
--cc=alex.aring@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=eric@anholt.net \
--cc=f.fainelli@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=ijc+devicetree@hellion.org.uk \
--cc=lee@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-rpi-kernel@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=pawel.moll@arm.com \
--cc=rjw@rjwysocki.net \
--cc=robh+dt@kernel.org \
--cc=swarren@wwwdotorg.org \
--cc=ulf.hansson@linaro.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.