* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-11-25 20:01 ` Kevin Bracey
0 siblings, 0 replies; 51+ messages in thread
From: Kevin Bracey @ 2013-11-25 20:01 UTC (permalink / raw)
To: Linus Walleij
Cc: Kyungmin Park, devicetree@vger.kernel.org, linux-samsung-soc,
Heiko Stübner, Stephen Warren, Tomasz Figa, Doug Anderson,
linux-kernel@vger.kernel.org, Kukjin Kim, Thomas Abraham,
linux-arm-kernel@lists.infradead.org, Marek Szyprowski
On 25/11/2013 16:34, Linus Walleij wrote:
> On Wed, Nov 20, 2013 at 1:02 AM, Kyungmin Park <kmpark@infradead.org> wrote:
>> On Wed, Nov 20, 2013 at 4:16 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> I think that last point should be addressed by having a driver that owns
>>> the GPIO set it to the desired output level, and the implementation of
>> Some pins are not connected (NC). At that cases, there's no drivers to
>> handle it. To reduce power leakage, it sets proper configuration with
>> values instead of reset values.
> This is correspondant to the PIN_CONFIG_OUTPUT from
> include/linux/pinctrl/pinconf-generic.h
Indeed it is - I was waiting for someone to point that out. Now we've
got there...
I've been working on extending the shmobile PFC driver to provide
"gpio-mode" and implement PIN_CONFIG_OUTPUT as described by
Documentation/pinctrl.txt, primarily to handle sleep states, but I have
begun to wonder about the initial state problem, as discussed here.
As far as I can see, we can't currently specify "fallback" states for
pins, which is one of Tomasz' key requirements.
We can specify "hog" states, and we can specify "default for a driver",
but not "default before/in absence of a driver" or "sleep in absence of
a driver". Having a hog precludes any finer driver control, AFAICT.
Some of our existing pre-pinconf board files have a "unused pins" table
which is used to claim and pull GPIOs. I've converted that to "hog"
pinconf, but that only works because the table omits all pins used by
drivers. And, unsurprisingly, that's been error-prone; if those tables
originally covered all unused pins, they don't any more.
I'd like confidence that we can get every pin into the correct state by
having a fully-populated table containing all pins, that can be used
regardless of which drivers start. I think what we're lacking is a "weak
hog". Whatever you call that. :)
That would also specify the state to fallback to when a group is
released (where we currently do pinmux_disable_setting).
Thoughts?
Kevin
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-11-25 20:01 ` Kevin Bracey
0 siblings, 0 replies; 51+ messages in thread
From: Kevin Bracey @ 2013-11-25 20:01 UTC (permalink / raw)
To: linux-arm-kernel
On 25/11/2013 16:34, Linus Walleij wrote:
> On Wed, Nov 20, 2013 at 1:02 AM, Kyungmin Park <kmpark@infradead.org> wrote:
>> On Wed, Nov 20, 2013 at 4:16 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> I think that last point should be addressed by having a driver that owns
>>> the GPIO set it to the desired output level, and the implementation of
>> Some pins are not connected (NC). At that cases, there's no drivers to
>> handle it. To reduce power leakage, it sets proper configuration with
>> values instead of reset values.
> This is correspondant to the PIN_CONFIG_OUTPUT from
> include/linux/pinctrl/pinconf-generic.h
Indeed it is - I was waiting for someone to point that out. Now we've
got there...
I've been working on extending the shmobile PFC driver to provide
"gpio-mode" and implement PIN_CONFIG_OUTPUT as described by
Documentation/pinctrl.txt, primarily to handle sleep states, but I have
begun to wonder about the initial state problem, as discussed here.
As far as I can see, we can't currently specify "fallback" states for
pins, which is one of Tomasz' key requirements.
We can specify "hog" states, and we can specify "default for a driver",
but not "default before/in absence of a driver" or "sleep in absence of
a driver". Having a hog precludes any finer driver control, AFAICT.
Some of our existing pre-pinconf board files have a "unused pins" table
which is used to claim and pull GPIOs. I've converted that to "hog"
pinconf, but that only works because the table omits all pins used by
drivers. And, unsurprisingly, that's been error-prone; if those tables
originally covered all unused pins, they don't any more.
I'd like confidence that we can get every pin into the correct state by
having a fully-populated table containing all pins, that can be used
regardless of which drivers start. I think what we're lacking is a "weak
hog". Whatever you call that. :)
That would also specify the state to fallback to when a group is
released (where we currently do pinmux_disable_setting).
Thoughts?
Kevin
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-11-25 20:01 ` Kevin Bracey
@ 2013-11-26 0:30 ` Tomasz Figa
-1 siblings, 0 replies; 51+ messages in thread
From: Tomasz Figa @ 2013-11-26 0:30 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Kevin Bracey, Linus Walleij, devicetree@vger.kernel.org,
linux-samsung-soc, Kyungmin Park, Heiko Stübner,
Stephen Warren, Tomasz Figa, Doug Anderson,
linux-kernel@vger.kernel.org, Kukjin Kim, Thomas Abraham,
Marek Szyprowski
On Monday 25 of November 2013 22:01:42 Kevin Bracey wrote:
> On 25/11/2013 16:34, Linus Walleij wrote:
> > On Wed, Nov 20, 2013 at 1:02 AM, Kyungmin Park <kmpark@infradead.org> wrote:
> >> On Wed, Nov 20, 2013 at 4:16 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >>> I think that last point should be addressed by having a driver that owns
> >>> the GPIO set it to the desired output level, and the implementation of
> >> Some pins are not connected (NC). At that cases, there's no drivers to
> >> handle it. To reduce power leakage, it sets proper configuration with
> >> values instead of reset values.
> > This is correspondant to the PIN_CONFIG_OUTPUT from
> > include/linux/pinctrl/pinconf-generic.h
>
> Indeed it is - I was waiting for someone to point that out. Now we've
> got there...
>
> I've been working on extending the shmobile PFC driver to provide
> "gpio-mode" and implement PIN_CONFIG_OUTPUT as described by
> Documentation/pinctrl.txt, primarily to handle sleep states, but I have
> begun to wonder about the initial state problem, as discussed here.
>
> As far as I can see, we can't currently specify "fallback" states for
> pins, which is one of Tomasz' key requirements.
>
> We can specify "hog" states, and we can specify "default for a driver",
> but not "default before/in absence of a driver" or "sleep in absence of
> a driver". Having a hog precludes any finer driver control, AFAICT.
>
> Some of our existing pre-pinconf board files have a "unused pins" table
> which is used to claim and pull GPIOs. I've converted that to "hog"
> pinconf, but that only works because the table omits all pins used by
> drivers. And, unsurprisingly, that's been error-prone; if those tables
> originally covered all unused pins, they don't any more.
>
> I'd like confidence that we can get every pin into the correct state by
> having a fully-populated table containing all pins, that can be used
> regardless of which drivers start. I think what we're lacking is a "weak
> hog". Whatever you call that. :)
>
> That would also specify the state to fallback to when a group is
> released (where we currently do pinmux_disable_setting).
>
> Thoughts?
I fully agree with Kevin. We're currently also using "hogs" for this, but
as Kevin mentioned, this is error prone, as it can be used for pins that
are not touched by any drivers.
IMHO a way to specify a default safe state of all pins (with lowest power
consumption, without possibility of glitching external devices, etc.)
would be really useful for Samsung platforms (and probably Renesas ones
as Kevin wrote).
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-11-26 0:30 ` Tomasz Figa
0 siblings, 0 replies; 51+ messages in thread
From: Tomasz Figa @ 2013-11-26 0:30 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 25 of November 2013 22:01:42 Kevin Bracey wrote:
> On 25/11/2013 16:34, Linus Walleij wrote:
> > On Wed, Nov 20, 2013 at 1:02 AM, Kyungmin Park <kmpark@infradead.org> wrote:
> >> On Wed, Nov 20, 2013 at 4:16 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >>> I think that last point should be addressed by having a driver that owns
> >>> the GPIO set it to the desired output level, and the implementation of
> >> Some pins are not connected (NC). At that cases, there's no drivers to
> >> handle it. To reduce power leakage, it sets proper configuration with
> >> values instead of reset values.
> > This is correspondant to the PIN_CONFIG_OUTPUT from
> > include/linux/pinctrl/pinconf-generic.h
>
> Indeed it is - I was waiting for someone to point that out. Now we've
> got there...
>
> I've been working on extending the shmobile PFC driver to provide
> "gpio-mode" and implement PIN_CONFIG_OUTPUT as described by
> Documentation/pinctrl.txt, primarily to handle sleep states, but I have
> begun to wonder about the initial state problem, as discussed here.
>
> As far as I can see, we can't currently specify "fallback" states for
> pins, which is one of Tomasz' key requirements.
>
> We can specify "hog" states, and we can specify "default for a driver",
> but not "default before/in absence of a driver" or "sleep in absence of
> a driver". Having a hog precludes any finer driver control, AFAICT.
>
> Some of our existing pre-pinconf board files have a "unused pins" table
> which is used to claim and pull GPIOs. I've converted that to "hog"
> pinconf, but that only works because the table omits all pins used by
> drivers. And, unsurprisingly, that's been error-prone; if those tables
> originally covered all unused pins, they don't any more.
>
> I'd like confidence that we can get every pin into the correct state by
> having a fully-populated table containing all pins, that can be used
> regardless of which drivers start. I think what we're lacking is a "weak
> hog". Whatever you call that. :)
>
> That would also specify the state to fallback to when a group is
> released (where we currently do pinmux_disable_setting).
>
> Thoughts?
I fully agree with Kevin. We're currently also using "hogs" for this, but
as Kevin mentioned, this is error prone, as it can be used for pins that
are not touched by any drivers.
IMHO a way to specify a default safe state of all pins (with lowest power
consumption, without possibility of glitching external devices, etc.)
would be really useful for Samsung platforms (and probably Renesas ones
as Kevin wrote).
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-11-26 0:30 ` Tomasz Figa
@ 2013-12-03 9:31 ` Linus Walleij
-1 siblings, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2013-12-03 9:31 UTC (permalink / raw)
To: Tomasz Figa
Cc: linux-arm-kernel@lists.infradead.org, Kevin Bracey,
devicetree@vger.kernel.org, linux-samsung-soc, Kyungmin Park,
Heiko Stübner, Stephen Warren, Tomasz Figa, Doug Anderson,
linux-kernel@vger.kernel.org, Kukjin Kim, Thomas Abraham,
Marek Szyprowski
On Tue, Nov 26, 2013 at 1:30 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> IMHO a way to specify a default safe state of all pins (with lowest power
> consumption, without possibility of glitching external devices, etc.)
> would be really useful for Samsung platforms (and probably Renesas ones
> as Kevin wrote).
I'm following :-)
Now if we could see some more details and a patch, that would
be great ...
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-03 9:31 ` Linus Walleij
0 siblings, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2013-12-03 9:31 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Nov 26, 2013 at 1:30 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> IMHO a way to specify a default safe state of all pins (with lowest power
> consumption, without possibility of glitching external devices, etc.)
> would be really useful for Samsung platforms (and probably Renesas ones
> as Kevin wrote).
I'm following :-)
Now if we could see some more details and a patch, that would
be great ...
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-12-03 9:31 ` Linus Walleij
@ 2013-12-03 9:33 ` Tomasz Figa
-1 siblings, 0 replies; 51+ messages in thread
From: Tomasz Figa @ 2013-12-03 9:33 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-arm-kernel@lists.infradead.org, Kevin Bracey,
devicetree@vger.kernel.org, linux-samsung-soc, Kyungmin Park,
Heiko Stübner, Stephen Warren, Tomasz Figa, Doug Anderson,
linux-kernel@vger.kernel.org, Kukjin Kim, Thomas Abraham,
Marek Szyprowski
On Tuesday 03 of December 2013 10:31:12 Linus Walleij wrote:
> On Tue, Nov 26, 2013 at 1:30 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>
> > IMHO a way to specify a default safe state of all pins (with lowest power
> > consumption, without possibility of glitching external devices, etc.)
> > would be really useful for Samsung platforms (and probably Renesas ones
> > as Kevin wrote).
>
> I'm following :-)
>
> Now if we could see some more details and a patch, that would
> be great ...
Yeah, I would like to see them too. ;)
Unfortunately my make_pinctrl_default_state_patches() function is
returning -ENOTIME at the moment. :( (I hope to get it fixed
later this month, though.)
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-03 9:33 ` Tomasz Figa
0 siblings, 0 replies; 51+ messages in thread
From: Tomasz Figa @ 2013-12-03 9:33 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 03 of December 2013 10:31:12 Linus Walleij wrote:
> On Tue, Nov 26, 2013 at 1:30 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>
> > IMHO a way to specify a default safe state of all pins (with lowest power
> > consumption, without possibility of glitching external devices, etc.)
> > would be really useful for Samsung platforms (and probably Renesas ones
> > as Kevin wrote).
>
> I'm following :-)
>
> Now if we could see some more details and a patch, that would
> be great ...
Yeah, I would like to see them too. ;)
Unfortunately my make_pinctrl_default_state_patches() function is
returning -ENOTIME at the moment. :( (I hope to get it fixed
later this month, though.)
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-11-25 20:01 ` Kevin Bracey
@ 2013-12-03 9:29 ` Linus Walleij
-1 siblings, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2013-12-03 9:29 UTC (permalink / raw)
To: Kevin Bracey
Cc: Kyungmin Park, devicetree@vger.kernel.org, linux-samsung-soc,
Heiko Stübner, Stephen Warren, Tomasz Figa, Doug Anderson,
linux-kernel@vger.kernel.org, Kukjin Kim, Thomas Abraham,
linux-arm-kernel@lists.infradead.org, Marek Szyprowski
On Mon, Nov 25, 2013 at 9:01 PM, Kevin Bracey <kevin@bracey.fi> wrote:
> I've been working on extending the shmobile PFC driver to provide
> "gpio-mode" and implement PIN_CONFIG_OUTPUT as described by
> Documentation/pinctrl.txt, primarily to handle sleep states, but I have
> begun to wonder about the initial state problem, as discussed here.
>
> As far as I can see, we can't currently specify "fallback" states for pins,
> which is one of Tomasz' key requirements.
That depends on what you mean with "fallback states" so let's hash
this out.
> We can specify "hog" states, and we can specify "default for a driver", but
> not "default before/in absence of a driver" or "sleep in absence of a
> driver". Having a hog precludes any finer driver control, AFAICT.
So the way it currently works is that hogs are usually used when
there is no corresponding driver that can take care of the pins.
If there are four SPI pins and no driver for that SPI block, a way
to e.g. ground them is to do so using pinctrl hogs.
Whenever a driver and a struct device * for this SPI block appears,
it is wise to transfer control of these over to the device core, which
will take any "default" state before doing probe() on the device
driver, and then give you the option to use the nice pinctrl_pm*
accessors to let the device core switch between "default" "idle"
and "sleep" states if these are defined.
> Some of our existing pre-pinconf board files have a "unused pins" table
> which is used to claim and pull GPIOs. I've converted that to "hog" pinconf,
> but that only works because the table omits all pins used by drivers. And,
> unsurprisingly, that's been error-prone; if those tables originally covered
> all unused pins, they don't any more.
>
> I'd like confidence that we can get every pin into the correct state by
> having a fully-populated table containing all pins, that can be used
> regardless of which drivers start. I think what we're lacking is a "weak
> hog". Whatever you call that. :)
>
> That would also specify the state to fallback to when a group is released
> (where we currently do pinmux_disable_setting).
So I guess what you're after is a kind of hog that will be pushed
aside and ignored if a struct device with an associated state appears
that will use the same pin?
It is true that we currently require the tables to be strict and not
overlap like this, so ideally you should remove the hogs when you
have a device driver, but you're actually describing an interesting
case here:
What if I have a driver for this IP block, and it was supposed to
take care of a few pins, but I decide not to compile it into my
kernel? Or if I have it as a module and only modprobe it later
at runtime?
So a suggested patch to support weak hogs would be interesting
to look at. Can you provide details on how you think this would
work?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-03 9:29 ` Linus Walleij
0 siblings, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2013-12-03 9:29 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Nov 25, 2013 at 9:01 PM, Kevin Bracey <kevin@bracey.fi> wrote:
> I've been working on extending the shmobile PFC driver to provide
> "gpio-mode" and implement PIN_CONFIG_OUTPUT as described by
> Documentation/pinctrl.txt, primarily to handle sleep states, but I have
> begun to wonder about the initial state problem, as discussed here.
>
> As far as I can see, we can't currently specify "fallback" states for pins,
> which is one of Tomasz' key requirements.
That depends on what you mean with "fallback states" so let's hash
this out.
> We can specify "hog" states, and we can specify "default for a driver", but
> not "default before/in absence of a driver" or "sleep in absence of a
> driver". Having a hog precludes any finer driver control, AFAICT.
So the way it currently works is that hogs are usually used when
there is no corresponding driver that can take care of the pins.
If there are four SPI pins and no driver for that SPI block, a way
to e.g. ground them is to do so using pinctrl hogs.
Whenever a driver and a struct device * for this SPI block appears,
it is wise to transfer control of these over to the device core, which
will take any "default" state before doing probe() on the device
driver, and then give you the option to use the nice pinctrl_pm*
accessors to let the device core switch between "default" "idle"
and "sleep" states if these are defined.
> Some of our existing pre-pinconf board files have a "unused pins" table
> which is used to claim and pull GPIOs. I've converted that to "hog" pinconf,
> but that only works because the table omits all pins used by drivers. And,
> unsurprisingly, that's been error-prone; if those tables originally covered
> all unused pins, they don't any more.
>
> I'd like confidence that we can get every pin into the correct state by
> having a fully-populated table containing all pins, that can be used
> regardless of which drivers start. I think what we're lacking is a "weak
> hog". Whatever you call that. :)
>
> That would also specify the state to fallback to when a group is released
> (where we currently do pinmux_disable_setting).
So I guess what you're after is a kind of hog that will be pushed
aside and ignored if a struct device with an associated state appears
that will use the same pin?
It is true that we currently require the tables to be strict and not
overlap like this, so ideally you should remove the hogs when you
have a device driver, but you're actually describing an interesting
case here:
What if I have a driver for this IP block, and it was supposed to
take care of a few pins, but I decide not to compile it into my
kernel? Or if I have it as a module and only modprobe it later
at runtime?
So a suggested patch to support weak hogs would be interesting
to look at. Can you provide details on how you think this would
work?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-12-03 9:29 ` Linus Walleij
@ 2013-12-05 15:07 ` Mark Brown
-1 siblings, 0 replies; 51+ messages in thread
From: Mark Brown @ 2013-12-05 15:07 UTC (permalink / raw)
To: Linus Walleij
Cc: Kevin Bracey, Kyungmin Park, devicetree@vger.kernel.org,
linux-samsung-soc, Heiko Stübner, Stephen Warren,
Tomasz Figa, Doug Anderson, linux-kernel@vger.kernel.org,
Kukjin Kim, Thomas Abraham, linux-arm-kernel@lists.infradead.org,
Marek Szyprowski
[-- Attachment #1: Type: text/plain, Size: 323 bytes --]
On Tue, Dec 03, 2013 at 10:29:42AM +0100, Linus Walleij wrote:
> So a suggested patch to support weak hogs would be interesting
> to look at. Can you provide details on how you think this would
> work?
Or should we be going and applying the default state to all devices on
init without worrying about a driver appearing?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-05 15:07 ` Mark Brown
0 siblings, 0 replies; 51+ messages in thread
From: Mark Brown @ 2013-12-05 15:07 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Dec 03, 2013 at 10:29:42AM +0100, Linus Walleij wrote:
> So a suggested patch to support weak hogs would be interesting
> to look at. Can you provide details on how you think this would
> work?
Or should we be going and applying the default state to all devices on
init without worrying about a driver appearing?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131205/e429e980/attachment-0001.sig>
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-12-05 15:07 ` Mark Brown
@ 2013-12-05 15:11 ` Tomasz Figa
-1 siblings, 0 replies; 51+ messages in thread
From: Tomasz Figa @ 2013-12-05 15:11 UTC (permalink / raw)
To: Mark Brown
Cc: Linus Walleij, Kevin Bracey, Kyungmin Park,
devicetree@vger.kernel.org, linux-samsung-soc, Heiko Stübner,
Stephen Warren, Doug Anderson, linux-kernel@vger.kernel.org,
Kukjin Kim, Thomas Abraham, linux-arm-kernel@lists.infradead.org,
Marek Szyprowski
On Thursday 05 of December 2013 15:07:47 Mark Brown wrote:
> On Tue, Dec 03, 2013 at 10:29:42AM +0100, Linus Walleij wrote:
>
> > So a suggested patch to support weak hogs would be interesting
> > to look at. Can you provide details on how you think this would
> > work?
>
> Or should we be going and applying the default state to all devices on
> init without worrying about a driver appearing?
If a device isn't used, then it's often better to configure the pins for
a different function, such as GPIO, to minimize leakage current.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-05 15:11 ` Tomasz Figa
0 siblings, 0 replies; 51+ messages in thread
From: Tomasz Figa @ 2013-12-05 15:11 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 05 of December 2013 15:07:47 Mark Brown wrote:
> On Tue, Dec 03, 2013 at 10:29:42AM +0100, Linus Walleij wrote:
>
> > So a suggested patch to support weak hogs would be interesting
> > to look at. Can you provide details on how you think this would
> > work?
>
> Or should we be going and applying the default state to all devices on
> init without worrying about a driver appearing?
If a device isn't used, then it's often better to configure the pins for
a different function, such as GPIO, to minimize leakage current.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-12-05 15:11 ` Tomasz Figa
(?)
@ 2013-12-05 16:49 ` Kevin Bracey
-1 siblings, 0 replies; 51+ messages in thread
From: Kevin Bracey @ 2013-12-05 16:49 UTC (permalink / raw)
To: Tomasz Figa
Cc: devicetree@vger.kernel.org, linux-samsung-soc, Kyungmin Park,
Heiko Stübner, Stephen Warren, Linus Walleij, Doug Anderson,
linux-kernel@vger.kernel.org, Mark Brown, Thomas Abraham,
Kukjin Kim, linux-arm-kernel@lists.infradead.org,
Marek Szyprowski
On 05/12/2013 17:11, Tomasz Figa wrote:
> On Thursday 05 of December 2013 15:07:47 Mark Brown wrote:
>> On Tue, Dec 03, 2013 at 10:29:42AM +0100, Linus Walleij wrote:
>>
>>> So a suggested patch to support weak hogs would be interesting
>>> to look at. Can you provide details on how you think this would
>>> work?
>> Or should we be going and applying the default state to all devices on
>> init without worrying about a driver appearing?
> If a device isn't used, then it's often better to configure the pins for
> a different function, such as GPIO, to minimize leakage current.
>
And there can also be mutually-exclusive drivers choosing different
default states for the same pin. I think you do need a separate "safe"
indicator.
My current thought is that a late-init "make safe all unclaimed pins"
pass would make sense - you can't really mess with pins in an automated
fashion on init, as it can mess up bootloader->driver handover. There
already exist late-init "turn off all unclaimed clocks" (at least on
shmobile) and "turn off all unclaimed regulators", and it would fit that
model.
Kevin
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-05 16:49 ` Kevin Bracey
0 siblings, 0 replies; 51+ messages in thread
From: Kevin Bracey @ 2013-12-05 16:49 UTC (permalink / raw)
To: Tomasz Figa
Cc: Mark Brown, Linus Walleij, Kyungmin Park,
devicetree@vger.kernel.org, linux-samsung-soc, Heiko Stübner,
Stephen Warren, Doug Anderson, linux-kernel@vger.kernel.org,
Kukjin Kim, Thomas Abraham, linux-arm-kernel@lists.infradead.org,
Marek Szyprowski
On 05/12/2013 17:11, Tomasz Figa wrote:
> On Thursday 05 of December 2013 15:07:47 Mark Brown wrote:
>> On Tue, Dec 03, 2013 at 10:29:42AM +0100, Linus Walleij wrote:
>>
>>> So a suggested patch to support weak hogs would be interesting
>>> to look at. Can you provide details on how you think this would
>>> work?
>> Or should we be going and applying the default state to all devices on
>> init without worrying about a driver appearing?
> If a device isn't used, then it's often better to configure the pins for
> a different function, such as GPIO, to minimize leakage current.
>
And there can also be mutually-exclusive drivers choosing different
default states for the same pin. I think you do need a separate "safe"
indicator.
My current thought is that a late-init "make safe all unclaimed pins"
pass would make sense - you can't really mess with pins in an automated
fashion on init, as it can mess up bootloader->driver handover. There
already exist late-init "turn off all unclaimed clocks" (at least on
shmobile) and "turn off all unclaimed regulators", and it would fit that
model.
Kevin
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-05 16:49 ` Kevin Bracey
0 siblings, 0 replies; 51+ messages in thread
From: Kevin Bracey @ 2013-12-05 16:49 UTC (permalink / raw)
To: linux-arm-kernel
On 05/12/2013 17:11, Tomasz Figa wrote:
> On Thursday 05 of December 2013 15:07:47 Mark Brown wrote:
>> On Tue, Dec 03, 2013 at 10:29:42AM +0100, Linus Walleij wrote:
>>
>>> So a suggested patch to support weak hogs would be interesting
>>> to look at. Can you provide details on how you think this would
>>> work?
>> Or should we be going and applying the default state to all devices on
>> init without worrying about a driver appearing?
> If a device isn't used, then it's often better to configure the pins for
> a different function, such as GPIO, to minimize leakage current.
>
And there can also be mutually-exclusive drivers choosing different
default states for the same pin. I think you do need a separate "safe"
indicator.
My current thought is that a late-init "make safe all unclaimed pins"
pass would make sense - you can't really mess with pins in an automated
fashion on init, as it can mess up bootloader->driver handover. There
already exist late-init "turn off all unclaimed clocks" (at least on
shmobile) and "turn off all unclaimed regulators", and it would fit that
model.
Kevin
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-12-05 16:49 ` Kevin Bracey
@ 2013-12-05 17:03 ` Tomasz Figa
-1 siblings, 0 replies; 51+ messages in thread
From: Tomasz Figa @ 2013-12-05 17:03 UTC (permalink / raw)
To: Kevin Bracey
Cc: Mark Brown, Linus Walleij, Kyungmin Park,
devicetree@vger.kernel.org, linux-samsung-soc, Heiko Stübner,
Stephen Warren, Doug Anderson, linux-kernel@vger.kernel.org,
Kukjin Kim, Thomas Abraham, linux-arm-kernel@lists.infradead.org,
Marek Szyprowski
On Thursday 05 of December 2013 18:49:56 Kevin Bracey wrote:
> On 05/12/2013 17:11, Tomasz Figa wrote:
> > On Thursday 05 of December 2013 15:07:47 Mark Brown wrote:
> >> On Tue, Dec 03, 2013 at 10:29:42AM +0100, Linus Walleij wrote:
> >>
> >>> So a suggested patch to support weak hogs would be interesting
> >>> to look at. Can you provide details on how you think this would
> >>> work?
> >> Or should we be going and applying the default state to all devices on
> >> init without worrying about a driver appearing?
> > If a device isn't used, then it's often better to configure the pins for
> > a different function, such as GPIO, to minimize leakage current.
> >
>
> And there can also be mutually-exclusive drivers choosing different
> default states for the same pin. I think you do need a separate "safe"
> indicator.
That's not quite true, as on a single board you should rather have
a single device node with "okay" status referencing given set of pins.
Still, I think that a separate safe state is the way to go.
>
> My current thought is that a late-init "make safe all unclaimed pins"
> pass would make sense - you can't really mess with pins in an automated
> fashion on init, as it can mess up bootloader->driver handover. There
> already exist late-init "turn off all unclaimed clocks" (at least on
> shmobile)
That's a feature of Common Clock Framework.
> and "turn off all unclaimed regulators", and it would fit that
> model.
Maybe that's the way to do it. I need to think a bit more on this,
especially considering our (Samsung's) use cases.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-05 17:03 ` Tomasz Figa
0 siblings, 0 replies; 51+ messages in thread
From: Tomasz Figa @ 2013-12-05 17:03 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 05 of December 2013 18:49:56 Kevin Bracey wrote:
> On 05/12/2013 17:11, Tomasz Figa wrote:
> > On Thursday 05 of December 2013 15:07:47 Mark Brown wrote:
> >> On Tue, Dec 03, 2013 at 10:29:42AM +0100, Linus Walleij wrote:
> >>
> >>> So a suggested patch to support weak hogs would be interesting
> >>> to look at. Can you provide details on how you think this would
> >>> work?
> >> Or should we be going and applying the default state to all devices on
> >> init without worrying about a driver appearing?
> > If a device isn't used, then it's often better to configure the pins for
> > a different function, such as GPIO, to minimize leakage current.
> >
>
> And there can also be mutually-exclusive drivers choosing different
> default states for the same pin. I think you do need a separate "safe"
> indicator.
That's not quite true, as on a single board you should rather have
a single device node with "okay" status referencing given set of pins.
Still, I think that a separate safe state is the way to go.
>
> My current thought is that a late-init "make safe all unclaimed pins"
> pass would make sense - you can't really mess with pins in an automated
> fashion on init, as it can mess up bootloader->driver handover. There
> already exist late-init "turn off all unclaimed clocks" (at least on
> shmobile)
That's a feature of Common Clock Framework.
> and "turn off all unclaimed regulators", and it would fit that
> model.
Maybe that's the way to do it. I need to think a bit more on this,
especially considering our (Samsung's) use cases.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-12-05 15:11 ` Tomasz Figa
@ 2013-12-05 18:00 ` Mark Brown
-1 siblings, 0 replies; 51+ messages in thread
From: Mark Brown @ 2013-12-05 18:00 UTC (permalink / raw)
To: Tomasz Figa
Cc: Linus Walleij, Kevin Bracey, Kyungmin Park,
devicetree@vger.kernel.org, linux-samsung-soc, Heiko Stübner,
Stephen Warren, Doug Anderson, linux-kernel@vger.kernel.org,
Kukjin Kim, Thomas Abraham, linux-arm-kernel@lists.infradead.org,
Marek Szyprowski
[-- Attachment #1: Type: text/plain, Size: 685 bytes --]
On Thu, Dec 05, 2013 at 04:11:15PM +0100, Tomasz Figa wrote:
> On Thursday 05 of December 2013 15:07:47 Mark Brown wrote:
> > Or should we be going and applying the default state to all devices on
> > init without worrying about a driver appearing?
> If a device isn't used, then it's often better to configure the pins for
> a different function, such as GPIO, to minimize leakage current.
That's true but on the other hand it's much more likely that the default
information will be there than the hog for the case when the driver
doesn't get loaded for some reason so it's more likely that the default
will kick in; perhaps we need to work down a list of states in order
instead?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-05 18:00 ` Mark Brown
0 siblings, 0 replies; 51+ messages in thread
From: Mark Brown @ 2013-12-05 18:00 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 05, 2013 at 04:11:15PM +0100, Tomasz Figa wrote:
> On Thursday 05 of December 2013 15:07:47 Mark Brown wrote:
> > Or should we be going and applying the default state to all devices on
> > init without worrying about a driver appearing?
> If a device isn't used, then it's often better to configure the pins for
> a different function, such as GPIO, to minimize leakage current.
That's true but on the other hand it's much more likely that the default
information will be there than the hog for the case when the driver
doesn't get loaded for some reason so it's more likely that the default
will kick in; perhaps we need to work down a list of states in order
instead?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131205/979ddebf/attachment.sig>
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-12-05 15:07 ` Mark Brown
@ 2013-12-09 10:22 ` Linus Walleij
-1 siblings, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2013-12-09 10:22 UTC (permalink / raw)
To: Mark Brown
Cc: Kevin Bracey, Kyungmin Park, devicetree@vger.kernel.org,
linux-samsung-soc, Heiko Stübner, Stephen Warren,
Tomasz Figa, Doug Anderson, linux-kernel@vger.kernel.org,
Kukjin Kim, Thomas Abraham, linux-arm-kernel@lists.infradead.org,
Marek Szyprowski
On Thu, Dec 5, 2013 at 4:07 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Dec 03, 2013 at 10:29:42AM +0100, Linus Walleij wrote:
>
>> So a suggested patch to support weak hogs would be interesting
>> to look at. Can you provide details on how you think this would
>> work?
>
> Or should we be going and applying the default state to all devices on
> init without worrying about a driver appearing?
That doesn't really work: if you have an unused device not in the
DT you usually mark it "disabled", and then the DT core doesn't
even create a platform_device for this node.
So doing this would involve parsing the tree and ... yuck.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-09 10:22 ` Linus Walleij
0 siblings, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2013-12-09 10:22 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 5, 2013 at 4:07 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Dec 03, 2013 at 10:29:42AM +0100, Linus Walleij wrote:
>
>> So a suggested patch to support weak hogs would be interesting
>> to look at. Can you provide details on how you think this would
>> work?
>
> Or should we be going and applying the default state to all devices on
> init without worrying about a driver appearing?
That doesn't really work: if you have an unused device not in the
DT you usually mark it "disabled", and then the DT core doesn't
even create a platform_device for this node.
So doing this would involve parsing the tree and ... yuck.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-12-09 10:22 ` Linus Walleij
@ 2013-12-09 17:04 ` Mark Brown
-1 siblings, 0 replies; 51+ messages in thread
From: Mark Brown @ 2013-12-09 17:04 UTC (permalink / raw)
To: Linus Walleij
Cc: Kevin Bracey, Kyungmin Park, devicetree@vger.kernel.org,
linux-samsung-soc, Heiko Stübner, Stephen Warren,
Tomasz Figa, Doug Anderson, linux-kernel@vger.kernel.org,
Kukjin Kim, Thomas Abraham, linux-arm-kernel@lists.infradead.org,
Marek Szyprowski
[-- Attachment #1: Type: text/plain, Size: 603 bytes --]
On Mon, Dec 09, 2013 at 11:22:44AM +0100, Linus Walleij wrote:
> On Thu, Dec 5, 2013 at 4:07 PM, Mark Brown <broonie@kernel.org> wrote:
> > Or should we be going and applying the default state to all devices on
> > init without worrying about a driver appearing?
> That doesn't really work: if you have an unused device not in the
> DT you usually mark it "disabled", and then the DT core doesn't
> even create a platform_device for this node.
> So doing this would involve parsing the tree and ... yuck.
Hrm, yeah, good point. That's got a low awesomeness factor even though
it should work. Boo.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-09 17:04 ` Mark Brown
0 siblings, 0 replies; 51+ messages in thread
From: Mark Brown @ 2013-12-09 17:04 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Dec 09, 2013 at 11:22:44AM +0100, Linus Walleij wrote:
> On Thu, Dec 5, 2013 at 4:07 PM, Mark Brown <broonie@kernel.org> wrote:
> > Or should we be going and applying the default state to all devices on
> > init without worrying about a driver appearing?
> That doesn't really work: if you have an unused device not in the
> DT you usually mark it "disabled", and then the DT core doesn't
> even create a platform_device for this node.
> So doing this would involve parsing the tree and ... yuck.
Hrm, yeah, good point. That's got a low awesomeness factor even though
it should work. Boo.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131209/075f08bc/attachment.sig>
^ permalink raw reply [flat|nested] 51+ messages in thread
[parent not found: <CACRpkdYrPd9tnO48bA_5n+-k=KEDyhY7czkCNhTZfpgF3ar+LQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-12-03 9:29 ` Linus Walleij
(?)
@ 2013-12-05 23:54 ` Stephen Warren
-1 siblings, 0 replies; 51+ messages in thread
From: Stephen Warren @ 2013-12-05 23:54 UTC (permalink / raw)
To: Linus Walleij, Kevin Bracey
Cc: Kyungmin Park, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-samsung-soc, Heiko Stübner, Tomasz Figa, Doug Anderson,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kukjin Kim,
Thomas Abraham,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Marek Szyprowski
On 12/03/2013 02:29 AM, Linus Walleij wrote:
...
> So I guess what you're after is a kind of hog that will be pushed
> aside and ignored if a struct device with an associated state appears
> that will use the same pin?
That probably would be useful. Perhaps we should just make all hogs not
exclusively own the pins they configure?
> It is true that we currently require the tables to be strict and not
> overlap like this, so ideally you should remove the hogs when you
> have a device driver, but you're actually describing an interesting
> case here:
>
> What if I have a driver for this IP block, and it was supposed to
> take care of a few pins, but I decide not to compile it into my
> kernel? Or if I have it as a module and only modprobe it later
> at runtime?
Indeed, that's the nasty hole in pushing even static per-device pinctrl
configuration into each device's node; the device may not appear.
Related, I prefer to put /all/ static pinctrl configuration into the
pinctrl device's "default" state (i.e. use a hog) rather than
configuring the static pinctrl setup per device, for another reason too:
If a particular IO controller's signals can be routed to n different
(sets of) pins, then we need to do *both* of the following when setting
up the pinmux:
a) Configure the pins we want to host those signals to route to/from
that particular IO controller.
b) Configure any other pins that could route to/from that particular IO
controller as some other function; either disabled, or routed to/from
some different IO controller.
That is so that the IO controller's RX/input signals are not connected
from two different sets of pins at once, which would cause two things to
driver them. Depending on HW, this could cause on of:
1) Multiple drivers -> high power usage, or even Silicon damage.
2) Inconsistent configuration, with the "wrong" set of pins driving the
IO controller's inputs, and hence the signals on the "correct" pins
being ignored -> hard to find bug.
Now, (a) could easily happen when the driver for the IO controller is
probed. However, (b) can't, because some other IO controller may need to
use those pins, and the two drivers (or pinctrl states for different
devices) can't both set up those pins.
The only way to solve this is to set up all pinmux state using a single
global table (e.g. the pin controller's default state, or hog) that is
applied early on.
If we rely on resolving these conflicts in per-device default/...
states, then that means the conflicts won't get resolved until a driver
gets probed, if it ever does, which is too late.
... and as such, I prefer only putting *dynamic* configuration into
per-device (non-hog) nodes. (e.g. an I2C bus mux driver which actively
changes the pinmux at run-time to move an I2C controller between
different sets of SoC pins).
--
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
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-05 23:54 ` Stephen Warren
0 siblings, 0 replies; 51+ messages in thread
From: Stephen Warren @ 2013-12-05 23:54 UTC (permalink / raw)
To: Linus Walleij, Kevin Bracey
Cc: Kyungmin Park, devicetree@vger.kernel.org, linux-samsung-soc,
Heiko Stübner, Tomasz Figa, Doug Anderson,
linux-kernel@vger.kernel.org, Kukjin Kim, Thomas Abraham,
linux-arm-kernel@lists.infradead.org, Marek Szyprowski
On 12/03/2013 02:29 AM, Linus Walleij wrote:
...
> So I guess what you're after is a kind of hog that will be pushed
> aside and ignored if a struct device with an associated state appears
> that will use the same pin?
That probably would be useful. Perhaps we should just make all hogs not
exclusively own the pins they configure?
> It is true that we currently require the tables to be strict and not
> overlap like this, so ideally you should remove the hogs when you
> have a device driver, but you're actually describing an interesting
> case here:
>
> What if I have a driver for this IP block, and it was supposed to
> take care of a few pins, but I decide not to compile it into my
> kernel? Or if I have it as a module and only modprobe it later
> at runtime?
Indeed, that's the nasty hole in pushing even static per-device pinctrl
configuration into each device's node; the device may not appear.
Related, I prefer to put /all/ static pinctrl configuration into the
pinctrl device's "default" state (i.e. use a hog) rather than
configuring the static pinctrl setup per device, for another reason too:
If a particular IO controller's signals can be routed to n different
(sets of) pins, then we need to do *both* of the following when setting
up the pinmux:
a) Configure the pins we want to host those signals to route to/from
that particular IO controller.
b) Configure any other pins that could route to/from that particular IO
controller as some other function; either disabled, or routed to/from
some different IO controller.
That is so that the IO controller's RX/input signals are not connected
from two different sets of pins at once, which would cause two things to
driver them. Depending on HW, this could cause on of:
1) Multiple drivers -> high power usage, or even Silicon damage.
2) Inconsistent configuration, with the "wrong" set of pins driving the
IO controller's inputs, and hence the signals on the "correct" pins
being ignored -> hard to find bug.
Now, (a) could easily happen when the driver for the IO controller is
probed. However, (b) can't, because some other IO controller may need to
use those pins, and the two drivers (or pinctrl states for different
devices) can't both set up those pins.
The only way to solve this is to set up all pinmux state using a single
global table (e.g. the pin controller's default state, or hog) that is
applied early on.
If we rely on resolving these conflicts in per-device default/...
states, then that means the conflicts won't get resolved until a driver
gets probed, if it ever does, which is too late.
... and as such, I prefer only putting *dynamic* configuration into
per-device (non-hog) nodes. (e.g. an I2C bus mux driver which actively
changes the pinmux at run-time to move an I2C controller between
different sets of SoC pins).
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-05 23:54 ` Stephen Warren
0 siblings, 0 replies; 51+ messages in thread
From: Stephen Warren @ 2013-12-05 23:54 UTC (permalink / raw)
To: linux-arm-kernel
On 12/03/2013 02:29 AM, Linus Walleij wrote:
...
> So I guess what you're after is a kind of hog that will be pushed
> aside and ignored if a struct device with an associated state appears
> that will use the same pin?
That probably would be useful. Perhaps we should just make all hogs not
exclusively own the pins they configure?
> It is true that we currently require the tables to be strict and not
> overlap like this, so ideally you should remove the hogs when you
> have a device driver, but you're actually describing an interesting
> case here:
>
> What if I have a driver for this IP block, and it was supposed to
> take care of a few pins, but I decide not to compile it into my
> kernel? Or if I have it as a module and only modprobe it later
> at runtime?
Indeed, that's the nasty hole in pushing even static per-device pinctrl
configuration into each device's node; the device may not appear.
Related, I prefer to put /all/ static pinctrl configuration into the
pinctrl device's "default" state (i.e. use a hog) rather than
configuring the static pinctrl setup per device, for another reason too:
If a particular IO controller's signals can be routed to n different
(sets of) pins, then we need to do *both* of the following when setting
up the pinmux:
a) Configure the pins we want to host those signals to route to/from
that particular IO controller.
b) Configure any other pins that could route to/from that particular IO
controller as some other function; either disabled, or routed to/from
some different IO controller.
That is so that the IO controller's RX/input signals are not connected
from two different sets of pins at once, which would cause two things to
driver them. Depending on HW, this could cause on of:
1) Multiple drivers -> high power usage, or even Silicon damage.
2) Inconsistent configuration, with the "wrong" set of pins driving the
IO controller's inputs, and hence the signals on the "correct" pins
being ignored -> hard to find bug.
Now, (a) could easily happen when the driver for the IO controller is
probed. However, (b) can't, because some other IO controller may need to
use those pins, and the two drivers (or pinctrl states for different
devices) can't both set up those pins.
The only way to solve this is to set up all pinmux state using a single
global table (e.g. the pin controller's default state, or hog) that is
applied early on.
If we rely on resolving these conflicts in per-device default/...
states, then that means the conflicts won't get resolved until a driver
gets probed, if it ever does, which is too late.
... and as such, I prefer only putting *dynamic* configuration into
per-device (non-hog) nodes. (e.g. an I2C bus mux driver which actively
changes the pinmux at run-time to move an I2C controller between
different sets of SoC pins).
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
2013-12-05 23:54 ` Stephen Warren
@ 2013-12-09 12:57 ` Linus Walleij
-1 siblings, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2013-12-09 12:57 UTC (permalink / raw)
To: Stephen Warren
Cc: Kevin Bracey, Kyungmin Park, devicetree@vger.kernel.org,
linux-samsung-soc, Heiko Stübner, Tomasz Figa, Doug Anderson,
linux-kernel@vger.kernel.org, Kukjin Kim, Thomas Abraham,
linux-arm-kernel@lists.infradead.org, Marek Szyprowski
On Fri, Dec 6, 2013 at 12:54 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 12/03/2013 02:29 AM, Linus Walleij wrote:
(skipped the conversation on weak hogs, we are on the same page
here, just waiting for someone to start working on it ...)
> Related, I prefer to put /all/ static pinctrl configuration into the
> pinctrl device's "default" state (i.e. use a hog) rather than
> configuring the static pinctrl setup per device, for another reason too:
>
> If a particular IO controller's signals can be routed to n different
> (sets of) pins, then we need to do *both* of the following when setting
> up the pinmux:
>
> a) Configure the pins we want to host those signals to route to/from
> that particular IO controller.
>
> b) Configure any other pins that could route to/from that particular IO
> controller as some other function; either disabled, or routed to/from
> some different IO controller.
>
> That is so that the IO controller's RX/input signals are not connected
> from two different sets of pins at once, which would cause two things to
> driver them. Depending on HW, this could cause on of:
>
> 1) Multiple drivers -> high power usage, or even Silicon damage.
>
> 2) Inconsistent configuration, with the "wrong" set of pins driving the
> IO controller's inputs, and hence the signals on the "correct" pins
> being ignored -> hard to find bug.
I'm following, I think what we need here is to think about additional
behaviours and electronic constraints we can encode into the drivers
and/or the pin tables to safeguard pin states from electronically
unsound states.
That is to say, I prefer the subsystem to be conscious about the
electronic constraints and navigate around them or put up a road
block, rather than trying ti avoid driving into the roadblocks by means
of carefully crafted tables if you get the picture ...
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH] pinctrl: samsung: Allow pin value to be initialized using pinfunc.
@ 2013-12-09 12:57 ` Linus Walleij
0 siblings, 0 replies; 51+ messages in thread
From: Linus Walleij @ 2013-12-09 12:57 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Dec 6, 2013 at 12:54 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 12/03/2013 02:29 AM, Linus Walleij wrote:
(skipped the conversation on weak hogs, we are on the same page
here, just waiting for someone to start working on it ...)
> Related, I prefer to put /all/ static pinctrl configuration into the
> pinctrl device's "default" state (i.e. use a hog) rather than
> configuring the static pinctrl setup per device, for another reason too:
>
> If a particular IO controller's signals can be routed to n different
> (sets of) pins, then we need to do *both* of the following when setting
> up the pinmux:
>
> a) Configure the pins we want to host those signals to route to/from
> that particular IO controller.
>
> b) Configure any other pins that could route to/from that particular IO
> controller as some other function; either disabled, or routed to/from
> some different IO controller.
>
> That is so that the IO controller's RX/input signals are not connected
> from two different sets of pins at once, which would cause two things to
> driver them. Depending on HW, this could cause on of:
>
> 1) Multiple drivers -> high power usage, or even Silicon damage.
>
> 2) Inconsistent configuration, with the "wrong" set of pins driving the
> IO controller's inputs, and hence the signals on the "correct" pins
> being ignored -> hard to find bug.
I'm following, I think what we need here is to think about additional
behaviours and electronic constraints we can encode into the drivers
and/or the pin tables to safeguard pin states from electronically
unsound states.
That is to say, I prefer the subsystem to be conscious about the
electronic constraints and navigate around them or put up a road
block, rather than trying ti avoid driving into the roadblocks by means
of carefully crafted tables if you get the picture ...
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 51+ messages in thread