All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
To: Kuninori Morimoto
	<kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Linux-ALSA <alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org>,
	Linux-DT <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Michael Turquette
	<mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>,
	Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>,
	Linux-Kernel
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux-ARM
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: [alsa-devel] [PATCH v2] clkdev: add devm_of_clk_get()
Date: Tue, 29 Nov 2016 13:05:56 -0800	[thread overview]
Message-ID: <20161129210556.GC6095@codeaurora.org> (raw)
In-Reply-To: <87y409cw71.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

On 11/24, Kuninori Morimoto wrote:
> 
> Hi Stephen, again
> 
> > > I've seen bindings that have the 'clocks' property at the top
> > > level and the appropriate 'clock-names' property to relate the
> > > clocks to a subnode.
> > > 
> > >  	sound_soc {
> > > 		clocks = <&xxx>, <&xxx>;
> > > 		clock-names = "cpu", "codec";
> > >  		...
> > >  		cpu {
> > >  			...
> > >  		};
> > >  		codec {
> > >  			...
> > >  		};
> > >  	};
> > > 
> > > Then the subnodes call clk_get() with the top level device and
> > > the name of their node and things match up. I suppose this
> > > binding is finalized though, so we can't really do that?
> > > 
> > > I see that the gpio framework has a similar design called
> > > devm_get_gpiod_from_child(), so how about we add a
> > > devm_get_clk_from_child() API? That would more closely match the
> > > intent here, which is to restrict the clk_get() operation to
> > > child nodes of the device passed as the first argument.
> > > 
> > > struct clk *devm_get_clk_from_child(struct device *dev,
> > > 				    const char *con_id,
> > > 				    struct device_node *child);
> 
> Thanks, but, my point is that Linux already have "of_clk_get()",
> but we don't have its devm_ version.
> The point is that of_clk_get() can get clock from "device_node".
> Why having devm_ version become so problem ?

The problem is that it encourages the use of of_clk_get() when
clk_get() is more desirable. Ideally of_clk_get() is never used
when a device exists. In this case, it seems like we need to
support it though, hence the suggestion of having a
devm_get_clk_from_child() API, that explicitly reads as "get a
clock from a child node of this device". The distinction is
important, because of_clk_get() should rarely be used.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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: Stephen Boyd <sboyd@codeaurora.org>
To: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Rob Herring <robh@kernel.org>,
	Linux-ALSA <alsa-devel@alsa-project.org>,
	Linux-DT <devicetree@vger.kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Russell King <linux@armlinux.org.uk>,
	Linux-Kernel <linux-kernel@vger.kernel.org>,
	Mark Brown <broonie@kernel.org>,
	linux-clk@vger.kernel.org,
	Linux-ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [alsa-devel] [PATCH v2] clkdev: add devm_of_clk_get()
Date: Tue, 29 Nov 2016 13:05:56 -0800	[thread overview]
Message-ID: <20161129210556.GC6095@codeaurora.org> (raw)
In-Reply-To: <87y409cw71.wl%kuninori.morimoto.gx@renesas.com>

On 11/24, Kuninori Morimoto wrote:
> 
> Hi Stephen, again
> 
> > > I've seen bindings that have the 'clocks' property at the top
> > > level and the appropriate 'clock-names' property to relate the
> > > clocks to a subnode.
> > > 
> > >  	sound_soc {
> > > 		clocks = <&xxx>, <&xxx>;
> > > 		clock-names = "cpu", "codec";
> > >  		...
> > >  		cpu {
> > >  			...
> > >  		};
> > >  		codec {
> > >  			...
> > >  		};
> > >  	};
> > > 
> > > Then the subnodes call clk_get() with the top level device and
> > > the name of their node and things match up. I suppose this
> > > binding is finalized though, so we can't really do that?
> > > 
> > > I see that the gpio framework has a similar design called
> > > devm_get_gpiod_from_child(), so how about we add a
> > > devm_get_clk_from_child() API? That would more closely match the
> > > intent here, which is to restrict the clk_get() operation to
> > > child nodes of the device passed as the first argument.
> > > 
> > > struct clk *devm_get_clk_from_child(struct device *dev,
> > > 				    const char *con_id,
> > > 				    struct device_node *child);
> 
> Thanks, but, my point is that Linux already have "of_clk_get()",
> but we don't have its devm_ version.
> The point is that of_clk_get() can get clock from "device_node".
> Why having devm_ version become so problem ?

The problem is that it encourages the use of of_clk_get() when
clk_get() is more desirable. Ideally of_clk_get() is never used
when a device exists. In this case, it seems like we need to
support it though, hence the suggestion of having a
devm_get_clk_from_child() API, that explicitly reads as "get a
clock from a child node of this device". The distinction is
important, because of_clk_get() should rarely be used.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

WARNING: multiple messages have this Message-ID (diff)
From: sboyd@codeaurora.org (Stephen Boyd)
To: linux-arm-kernel@lists.infradead.org
Subject: [alsa-devel] [PATCH v2] clkdev: add devm_of_clk_get()
Date: Tue, 29 Nov 2016 13:05:56 -0800	[thread overview]
Message-ID: <20161129210556.GC6095@codeaurora.org> (raw)
In-Reply-To: <87y409cw71.wl%kuninori.morimoto.gx@renesas.com>

On 11/24, Kuninori Morimoto wrote:
> 
> Hi Stephen, again
> 
> > > I've seen bindings that have the 'clocks' property at the top
> > > level and the appropriate 'clock-names' property to relate the
> > > clocks to a subnode.
> > > 
> > >  	sound_soc {
> > > 		clocks = <&xxx>, <&xxx>;
> > > 		clock-names = "cpu", "codec";
> > >  		...
> > >  		cpu {
> > >  			...
> > >  		};
> > >  		codec {
> > >  			...
> > >  		};
> > >  	};
> > > 
> > > Then the subnodes call clk_get() with the top level device and
> > > the name of their node and things match up. I suppose this
> > > binding is finalized though, so we can't really do that?
> > > 
> > > I see that the gpio framework has a similar design called
> > > devm_get_gpiod_from_child(), so how about we add a
> > > devm_get_clk_from_child() API? That would more closely match the
> > > intent here, which is to restrict the clk_get() operation to
> > > child nodes of the device passed as the first argument.
> > > 
> > > struct clk *devm_get_clk_from_child(struct device *dev,
> > > 				    const char *con_id,
> > > 				    struct device_node *child);
> 
> Thanks, but, my point is that Linux already have "of_clk_get()",
> but we don't have its devm_ version.
> The point is that of_clk_get() can get clock from "device_node".
> Why having devm_ version become so problem ?

The problem is that it encourages the use of of_clk_get() when
clk_get() is more desirable. Ideally of_clk_get() is never used
when a device exists. In this case, it seems like we need to
support it though, hence the suggestion of having a
devm_get_clk_from_child() API, that explicitly reads as "get a
clock from a child node of this device". The distinction is
important, because of_clk_get() should rarely be used.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

  parent reply	other threads:[~2016-11-29 21:05 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-04  1:36 [PATCH v2] clkdev: add devm_of_clk_get() Kuninori Morimoto
2016-07-04  1:36 ` Kuninori Morimoto
2016-07-04  1:36 ` Kuninori Morimoto
2016-07-07  0:43 ` Michael Turquette
2016-07-07  0:43   ` Michael Turquette
2016-07-07  0:43   ` Michael Turquette
2016-07-07  0:43   ` Michael Turquette
2016-07-07  9:54   ` Kuninori Morimoto
2016-07-07  9:54     ` Kuninori Morimoto
2016-07-07  9:54     ` Kuninori Morimoto
2016-07-07 12:26     ` Russell King - ARM Linux
2016-07-07 12:26       ` Russell King - ARM Linux
2016-07-08  0:03       ` Kuninori Morimoto
2016-07-08  0:03         ` Kuninori Morimoto
2016-07-08  0:03         ` Kuninori Morimoto
2016-07-08  1:30         ` Michael Turquette
2016-07-08  1:30           ` Michael Turquette
2016-07-08  1:30           ` Michael Turquette
2016-07-08  3:18           ` Kuninori Morimoto
2016-07-08  3:18             ` Kuninori Morimoto
2016-07-08  3:18             ` Kuninori Morimoto
2016-07-27  0:51             ` [alsa-devel] " Kuninori Morimoto
2016-07-27  0:51               ` Kuninori Morimoto
2016-07-27  0:51               ` Kuninori Morimoto
2016-07-27  0:51             ` Kuninori Morimoto
2016-07-27  0:51               ` Kuninori Morimoto
2016-07-27  0:51               ` Kuninori Morimoto
2016-11-16  5:17               ` Kuninori Morimoto
2016-11-16  5:17                 ` Kuninori Morimoto
2016-11-16  5:17                 ` Kuninori Morimoto
2016-11-23 19:10                 ` Stephen Boyd
2016-11-23 19:10                   ` Stephen Boyd
     [not found]                   ` <20161123191037.GE25626-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-11-24  1:45                     ` Kuninori Morimoto
2016-11-24  1:45                       ` Kuninori Morimoto
2016-11-24  1:45                       ` Kuninori Morimoto
2016-11-24  4:57                       ` Kuninori Morimoto
2016-11-24  4:57                         ` Kuninori Morimoto
2016-11-24  4:57                         ` Kuninori Morimoto
     [not found]                         ` <87y409cw71.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
2016-11-29 21:05                           ` Stephen Boyd [this message]
2016-11-29 21:05                             ` Stephen Boyd
2016-11-29 21:05                             ` Stephen Boyd
     [not found]                             ` <20161129210556.GC6095-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-11-30  1:08                               ` Kuninori Morimoto
2016-11-30  1:08                                 ` Kuninori Morimoto
2016-11-30  1:08                                 ` Kuninori Morimoto
     [not found]                                 ` <874m2pbwsn.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
2016-12-01  1:43                                   ` Kuninori Morimoto
2016-12-01  1:43                                     ` Kuninori Morimoto
2016-12-01  1:43                                     ` Kuninori Morimoto

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=20161129210556.GC6095@codeaurora.org \
    --to=sboyd-sgv2jx0feol9jmxxk+q4oq@public.gmane.org \
    --cc=alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org \
    --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org \
    --cc=linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org \
    --cc=robh-DgEjT+Ai2ygdnm+yROfE0A@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.