Hello Vaibhav, Afzal, Vaibhav,
A few questions while reviewing this patch:
On Tue, 3 Apr 2012, Vaibhav Hiremath wrote:
> AM33XX PRCM module consist of, various clockdomains, in all
> total we have 18 clockdomains available, with following
> controlling options,
> - NO Sleep: sleep transition can not be initiated,
> - SW Sleep: sw forced sleep transition,
> - SW Wakeup: sw forced wakeup transition,
> - HW Auto: transitions are based upon hw conditions.
>
> This patch adds all available clockdomain data, respective
> clockdomain operations for AM33XX family of device, and also
> integrates it into existing OMAP framework.
>
> Signed-off-by: Vaibhav Hiremath <hvaibhav@xxxxxx>
> Signed-off-by: Afzal Mohammed <afzal@xxxxxx>
> Signed-off-by: Vaibhav Bedia <vaibhav.bedia@xxxxxx>
...
> +static struct clockdomain l4ls_am33xx_clkdm = {
> + .name = "l4ls_clkdm",
> + .pwrdm = { .name = "per_pwrdm" },
> + .cm_inst = AM33XX_CM_PER_MOD,
> + .clkdm_offs = AM33XX_CM_PER_L4LS_CLKSTCTRL_OFFSET,
> + .clktrctrl_mask = AM33XX_CLKTRCTRL_MASK,
> + .flags = (CLKDM_CAN_SWSUP | CLKDM_NO_AUTODEPS),
It looks to me like we don't need the .clktrctrl_mask field, since
according to the clockdomains code, the CLKTRCTRL field is at the same bit
shift for each clockdomain.
Also, since we're not defining any autodeps for the AM335x platform, we
shouldn't need the CLKDM_NO_AUTODEPS flag either? Since no autodeps are
defined, the default will be to not set any autodeps.
Another question is about the CLKTRCTRL bitfields. According to
_AM335x ARM Cortex-A8 Microprocessors (MPUs) Technical Reference
Manual_ Rev. D (SPRUH73D), most of the clockdomains support NO_SLEEP mode
(i.e., CLKTRCTRL = 0x0). That means that technically, we should also set
the CLKDM_CAN_DISABLE_AUTO flag. Unless the documentation is incorrect
here? In another section (Table 8-9 "Clock Transition Mode Settings"),
it claims that CLKTRCTRL = 0 is not supported...
Also, a question about the L4_WKUP clockdomain:
> +static struct clockdomain l4_wkup_am33xx_clkdm = {
> + .name = "l4_wkup_clkdm",
> + .pwrdm = { .name = "wkup_pwrdm" },
> + .cm_inst = AM33XX_CM_WKUP_MOD,
> + .clkdm_offs = AM33XX_CM_WKUP_CLKSTCTRL_OFFSET,
> + .clktrctrl_mask = AM33XX_CLKTRCTRL_MASK,
> + .flags = (CLKDM_CAN_SWSUP | CLKDM_NO_AUTODEPS),
> +};
Table 8-89 "CM_WKUP_CLKSTCTRL Register Field Descriptions" of SPRUH73D
claims that this clockdomain supports hardware-supervised automatic
clockdomain transitions. Again this conflicts with Table 8-9. Is this a
documentation bug, or should we update the patch?
- Paul
--
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
