RE: [RFC: PATCH] Fix to support multiple HWMODS for a single device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Cousson, Benoit
> Sent: Monday, August 23, 2010 5:20 PM
> To: ABRAHAM, KISHON VIJAY
> Cc: linux-omap@xxxxxxxxxxxxxxx; khilman@xxxxxxxxxxxxxxxxxxx;
> paul@xxxxxxxxx
> Subject: Re: [RFC: PATCH] Fix to support multiple HWMODS for a single
> device
> 
> Hi Vijay,
> 
> On 8/23/2010 12:46 PM, ABRAHAM, KISHON VIJAY wrote:
> > The current HWMOD code expects multiple HWMODS to be filled in
> consecutive
> > memory location before passing to omap_device_build_ss().
> 
> Just a minor comment: this is not the "The current HWMOD code" but the
> omap_device core code.
> 
> > Ignoring this
> > will result in incorrect HWMOD data being extracted. This means before
> calling
> > omap_device_build_ss() the user has to create memory chunks, copy all
> the HWMOD
> > structures to it taking care of the mutex.
> 
> I don't think it was an expectation, this is simply a bug :-)
> So copying hwmod structures before calling this API is clearly not the
> right way to do fix that...
> 
> > This fix uses the pointer to pointer to OMAP_HWMOD structure passed to
> > omap_device_build_ss() to correctly extract the appropriate
> > OMAP_HWMOD structure.
> 
> Yes, this is the proper way of fixing that.
> 
> > This patch is created on top of origin/pm-wip/hwmods-omap4.
> 
> You can do it on the mainline too, there is no dependency with lo/master
> or pm-wip for that one. That fix will be able to go to mainline faster.
> 
> >
> > Signed-off-by: Kishon Vijay Abraham I<kishon@xxxxxx>
> > ---
> >   arch/arm/plat-omap/omap_device.c |   42 +++++++++++++++++++-----------
> --------
> >   1 files changed, 21 insertions(+), 21 deletions(-)
> >
> > diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-
> omap/omap_device.c
> > index d2b1609..e94bd7a 100644
> > --- a/arch/arm/plat-omap/omap_device.c
> > +++ b/arch/arm/plat-omap/omap_device.c
> > @@ -257,12 +257,12 @@ static inline struct omap_device
> *_find_by_pdev(struct platform_device *pdev)
> >    */
> >   int omap_device_count_resources(struct omap_device *od)
> >   {
> > -	struct omap_hwmod *oh;
> > +	struct omap_hwmod **oh;
> >   	int c = 0;
> >   	int i;
> >
> > -	for (i = 0, oh = *od->hwmods; i<  od->hwmods_cnt; i++, oh++)
> > -		c += omap_hwmod_count_resources(oh);
> > +	for (i = 0, oh = od->hwmods; i<  od->hwmods_cnt; i++, oh++)
> > +		c += omap_hwmod_count_resources(*oh);
> 
> In that case, you might prefer using array type of access in order to
> make the code more readable (i.e. oh[i]). Or even avoid the oh variable.
> 
>   +		c += omap_hwmod_count_resources(od->hwmods[i]);
> 
> Just for my information, what kind of device are you working on that
> require multiple hwmods?


The McBSP qualifies for 2 hwmods and 1 device as OMAP3 has sidetone support also.

> 
> Otherwise, this is a good catch. Thanks for fixing that.
> 
> Regards,
> Benoit
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux