On Thu, Sep 06, 2012 at 10:33:32, Bedia, Vaibhav wrote:
> 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@xxxxxx> [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].
>
Still didn't make it to mainline :)
> 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().
>
It is doable, and I believe it is the only option we have currently unless
we add something newly.
Thanks,
Vaibhav
> 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@xxxxxxxxxxxxxxxxxxx
> > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
>
>
--
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
