linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: hvaibhav@ti.com (Vaibhav Hiremath)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to pass d_can raminit
Date: Thu, 6 Sep 2012 09:26:07 +0530	[thread overview]
Message-ID: <50481ED7.7050409@ti.com> (raw)
In-Reply-To: <20120905231837.GI1303@atomide.com>



On 9/6/2012 4:48 AM, Tony Lindgren wrote:
> Hi,
> 
> * AnilKumar Ch <anilkumar@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.

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
> 

  reply	other threads:[~2012-09-06  3:56 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-05 11:12 [PATCH v2 0/4] can: c_can: Add suspend/resume and pinctrl support AnilKumar Ch
2012-09-05 11:12 ` [PATCH RESEND 1/4] can: c_can: Adopt " AnilKumar Ch
2012-09-05 23:19   ` Tony Lindgren
2012-09-05 11:12 ` [PATCH RESEND 2/4] can: c_can: Add d_can raminit support AnilKumar Ch
2012-09-05 23:23   ` Tony Lindgren
2012-09-05 11:12 ` [PATCH RESEND 3/4] ARM: AM33XX: board-generic: Add of_dev_auxdata to pass d_can raminit AnilKumar Ch
2012-09-05 23:18   ` Tony Lindgren
2012-09-06  3:56     ` Vaibhav Hiremath [this message]
2012-09-06  5:03       ` Bedia, Vaibhav
2012-09-06  5:07         ` Hiremath, Vaibhav
2012-09-05 11:12 ` [PATCH v2 4/4] can: c_can: Add d_can suspend resume support AnilKumar Ch
2012-09-07  8:26   ` AnilKumar, Chimata
2012-09-05 11:54 ` [PATCH v2 0/4] can: c_can: Add suspend/resume and pinctrl support Marc Kleine-Budde
2012-09-05 13:55   ` Marc Kleine-Budde
2012-09-05 23:26     ` Tony Lindgren
2012-09-06  6:09       ` AnilKumar, Chimata
2012-09-06  8:27         ` Marc Kleine-Budde

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=50481ED7.7050409@ti.com \
    --to=hvaibhav@ti.com \
    --cc=linux-arm-kernel@lists.infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).