RE: [PATCH 04/13] OMAP: Introduce dependent voltage domain support

[Vishwanath Sripathy <vishwanath.bs@ti.com>]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Thursday, February 10, 2011 10:07 PM
> To: Vishwanath Sripathy
> Cc: linux-omap@vger.kernel.org; patches@linaro.org; Thara Gopinath
> Subject: Re: [PATCH 04/13] OMAP: Introduce dependent voltage domain
> support
>
> Vishwanath Sripathy <vishwanath.bs@ti.com> writes:
>
> [...]
>
> >> > diff --git a/arch/arm/mach-omap2/dvfs.c b/arch/arm/mach-
> >> omap2/dvfs.c
> >> > index cefc2be..c9d3894 100755
> >> > --- a/arch/arm/mach-omap2/dvfs.c
> >> > +++ b/arch/arm/mach-omap2/dvfs.c
> >> > @@ -85,6 +85,7 @@ struct omap_vdd_dvfs_info {
> >> >  	struct mutex scaling_mutex; /* dvfs mutex */
> >> >  	struct voltagedomain *voltdm;
> >> >  	struct list_head dev_list;
> >> > +	struct device vdd_device;
> >>
> >> It's not clear what the usage of this device is for.
> >>
> >> It is never initialized, but seems to be used as a dummy device when
> >> calcluating dependencies.   Needs clarification.
> >
> > This device is used for placing voltage request
> (omap_dvfs_add_vdd_user)
> > when dealing with dependent vdds. Eg: while scaling MPU VDD, we
> also scale
> > CORE VDD. So while placing voltage request for CORE VDD, this
> vdd_device
> > of MPU is used as the requesting device so that this request is stored
> as
> > a separate user.
>
> I was able to follow how it was used, and why.   But it is still not
> clear to the reader.
>
> First, the device is never initialized, so it's essentially a dummy
> device that remains initialized by default to all zeros.   So, any
> attempt to use any device APIs (e.g. dev_name, etc.) on this will not
> behave as expected.
>
> Second, the reason for this device and the need for the request to be
> stored as a separate user are not clear in the code and don't exist in
> the comments.
OK. I will add more comments to make these details more explicit.

Vishwa
>
> Kevin
>
>
>
>