[PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to pass d_can raminit

[Bedia, Vaibhav]

On Thu, Sep 06, 2012 at 09:26:07, Hiremath, Vaibhav wrote:
> 
> 
> On 9/6/2012 4:48 AM, Tony Lindgren wrote:
> > Hi,
> > 
> > * AnilKumar Ch <anilkumar at ti.com> [120905 04:14]:
> >> Add of_dev_auxdata to pass d_can raminit callback APIs to initialize
> >> d_can RAM. D_CAN RAM initialization bits are present in CONTROL module
> >> address space, which can be accessed by platform specific code. So
> >> callback functions are added to serve this purpose, this can done by
> >> using of_dev_auxdata.
> >>
> >> Two callback APIs are added to of_dev_auxdata used by two instances of
> >> D_CAN IP. These callback functions are used to enable/disable D_CAN RAM
> >> from CAN driver.
> > 
> > I'd like to avoid the callbacks to the platform code where possible as
> > that's the biggest pain we already have moving things to work with device
> > tree for the existing drivers.
> > 
> > And I'm pretty convinced that whatever is done with callbacks should be
> > done with some Linux generic framework from the driver that has it's own
> > binding, such as clock framework, regulator framework, pinctrl framework,
> > runtime PM etc.
> > 
> >> --- a/arch/arm/mach-omap2/board-generic.c
> >> +++ b/arch/arm/mach-omap2/board-generic.c
> >> @@ -37,11 +40,46 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
> >>  	{ }
> >>  };
> >>  
> >> +void d_can_hw_raminit(unsigned int instance, bool enable)
> >> +{
> >> +	u32 val;
> >> +
> >> +	val = readl(AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> >> +	if (enable) {
> >> +		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
> >> +		val |= AM33XX_DCAN_RAMINIT_START_MASK(instance);
> >> +		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> >> +	} else {
> >> +		val &= ~AM33XX_DCAN_RAMINIT_START_MASK(instance);
> >> +		writel(val, AM33XX_CTRL_REGADDR(AM33XX_CONTROL_DCAN_RAMINIT));
> >> +	}
> >> +}
> > 
> > This part does not look good to me, this is tweaking the omap control
> > module register bits directly. To me it seems that the above should
> > be implemented in the omap/am33xx hwmod code that gets initialized when
> > the dcan driver calls pm_runtime_enable()? Paul, got any other ideas?
> > 
> 
> Technically yes, this is required during module enable/disable sequence.
> But there is no way currently supported in hwmod layer. Also I am not
> quite sure how many other modules/devices may use this.
> 

It should be possible to do this by providing custom activate/deactivate
functions similar to what was done for EMAC on AM35x [1].

However, right now on DT based systems omap_device_alloc() is called without
any pm_lats

	od = omap_device_alloc(pdev, hwmods, oh_cnt, NULL, 0);

The function pointers from the of_dev_auxdata somehow needs to be passed to
omap_device_alloc().

Regards,
Vaibhav B.

> Couple of more examples I have here,
> 
> In case of AM3517 we have similar SoC integration, where VPFE, MAC and
> USB required clock control (handled by clock-tree) and interrupt status
> (handled by callbacks) from control module.
> So not sure whether we can get rid of callbacks until we have control
> module MFD driver (on which Konstantin is working on)
> 
> Thanks,
> Vaibhav
> 
> > Regards,
> > 
> > Tony
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>