All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
To: Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org>
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] i2c-mux-pca954x: Disable mux after 200ms timeout
Date: Tue, 26 Nov 2013 18:00:19 +0100	[thread overview]
Message-ID: <20131126170019.GA18696@katana> (raw)
In-Reply-To: <5294AADC.5070707-Oq418RWZeHk@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]


> I had looked a bit in that direction, but I think there's currently
> no way for a driver to say "I won't be needing the bus for a while".
> Something like that would be critical for such a pm system to work.

Yes. I wasn't sure if something already existed.

> In any case, it doesn't sound like something I can sell - it's
> understandable for my employer that I spent an extra hour or so to
> clean up and submit the code to upstream, but this appears to go
> into a different class of rework.
>
> So where do you want to go with this? Should I rework the patch to
> make the timeout optional, or should I simply forget about
> integrating at all?

I understand your constraints, yet from a maintenance perspective I
shouldn't have such code in a driver. So, out-of-tree that is for now.

> In fact, on the customer's board, the pca mux is powered by a supply
> so the whole mux can be powered-down too, for which I also needed to
> patch the pca driver to reset its state when the powersupply
> reported that it was going down. I think that particular patch isn't
> of much use to the community though, or is it?

If it uses standard pm callbacks, I'd think this makes sense.

> Most hardware can control power and clocks to the I2C controller,
> which would indeed account for some power savings. But again, that
> would require drivers to provide estimations as to when they will
> need the bus. And it would require much more information on the bus
> controller too, though I suspect that to be the easier part.

There are drivers gating off the clocks simply after every transfer. I
don't know your HW details and workloads, but I wondered if you can
unconditionally switch off the core, do some pinmuxing...

> > Anyway, thanks for letting me know about your requirements (they should
> > have been in the original commit message, though ;))
> 
> I'm new to Linux kernel development, so please forgive me...

That was just a pointer, no complaint. You did a fine job in supplying
information around your patch, so thanks.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Wolfram Sang <wsa@the-dreams.de>
To: Mike Looijmans <mike.looijmans@topic.nl>
Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pm@vger.kernel.org
Subject: Re: [PATCH] i2c-mux-pca954x: Disable mux after 200ms timeout
Date: Tue, 26 Nov 2013 18:00:19 +0100	[thread overview]
Message-ID: <20131126170019.GA18696@katana> (raw)
In-Reply-To: <5294AADC.5070707@topic.nl>

[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]


> I had looked a bit in that direction, but I think there's currently
> no way for a driver to say "I won't be needing the bus for a while".
> Something like that would be critical for such a pm system to work.

Yes. I wasn't sure if something already existed.

> In any case, it doesn't sound like something I can sell - it's
> understandable for my employer that I spent an extra hour or so to
> clean up and submit the code to upstream, but this appears to go
> into a different class of rework.
>
> So where do you want to go with this? Should I rework the patch to
> make the timeout optional, or should I simply forget about
> integrating at all?

I understand your constraints, yet from a maintenance perspective I
shouldn't have such code in a driver. So, out-of-tree that is for now.

> In fact, on the customer's board, the pca mux is powered by a supply
> so the whole mux can be powered-down too, for which I also needed to
> patch the pca driver to reset its state when the powersupply
> reported that it was going down. I think that particular patch isn't
> of much use to the community though, or is it?

If it uses standard pm callbacks, I'd think this makes sense.

> Most hardware can control power and clocks to the I2C controller,
> which would indeed account for some power savings. But again, that
> would require drivers to provide estimations as to when they will
> need the bus. And it would require much more information on the bus
> controller too, though I suspect that to be the easier part.

There are drivers gating off the clocks simply after every transfer. I
don't know your HW details and workloads, but I wondered if you can
unconditionally switch off the core, do some pinmuxing...

> > Anyway, thanks for letting me know about your requirements (they should
> > have been in the original commit message, though ;))
> 
> I'm new to Linux kernel development, so please forgive me...

That was just a pointer, no complaint. You did a fine job in supplying
information around your patch, so thanks.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  parent reply	other threads:[~2013-11-26 17:00 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-26  6:32 [PATCH] i2c-mux-pca954x: Disable mux after 200ms timeout Mike Looijmans
2013-11-26  6:32 ` Mike Looijmans
     [not found] ` <1385447520-3306-1-git-send-email-mike.looijmans-Oq418RWZeHk@public.gmane.org>
2013-11-26  9:06   ` Wolfram Sang
2013-11-26  9:06     ` Wolfram Sang
2013-11-26  9:36     ` Mike Looijmans
2013-11-26  9:36       ` Mike Looijmans
2013-11-26 12:28       ` Wolfram Sang
2013-11-26 13:39         ` Ulf Hansson
2013-11-26 13:39           ` Ulf Hansson
2013-11-26 14:06         ` Mike Looijmans
2013-11-26 14:06           ` Mike Looijmans
     [not found]           ` <5294AADC.5070707-Oq418RWZeHk@public.gmane.org>
2013-11-26 17:00             ` Wolfram Sang [this message]
2013-11-26 17:00               ` Wolfram Sang
  -- strict thread matches above, loose matches on Subject: below --
2013-11-25 15:02 Mike Looijmans
2013-11-25 15:02 ` Mike Looijmans
2013-11-25 13:43 Mike Looijmans
2013-11-25 14:54 ` Michael Lawnick
2013-11-25 14:57 ` Wolfram Sang

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=20131126170019.GA18696@katana \
    --to=wsa-z923lk4zbo2bacvfa/9k2g@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mike.looijmans-Oq418RWZeHk@public.gmane.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.