lkml.org 
[lkml]   [2015]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH V3 2/2] spmi: pmic_arb: add support for hw version 2
Hey Gilad-

On Mon, Feb 09, 2015 at 03:51:12PM -0700, Gilad Avidov wrote:
> Qualcomm PMIC Arbiter version-2 changes from version-1 are:
>
> - Some different register offsets.
> - New channel register space, one per PMIC peripheral (ppid).
> All tx traffic uses these channels.
> - New observer register space. All rx trafic uses this space.
> - Different command format for spmi command registers.
>
> Acked-by: Sagar Dharia <sdharia@codeaurora.org>
> Signed-off-by: Gilad Avidov <gavidov@codeaurora.org>
[..]
> +++ b/drivers/spmi/spmi-pmic-arb.c
[..]
> @@ -645,12 +795,65 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
> pa->spmic = ctrl;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
> - pa->base = devm_ioremap_resource(&ctrl->dev, res);
> - if (IS_ERR(pa->base)) {
> - err = PTR_ERR(pa->base);
> + pa->rd_base = devm_ioremap_resource(&ctrl->dev, res);

This seems like an awkward way to do this, especially if you end up
remapping it...

> + if (IS_ERR(pa->rd_base)) {
> + err = PTR_ERR(pa->rd_base);
> goto err_put_ctrl;
> }
>
> + hw_ver = readl_relaxed(pa->rd_base + PMIC_ARB_VERSION);
> + is_v1 = (hw_ver < PMIC_ARB_VERSION_V2_MIN);
> +
> + dev_info(&ctrl->dev, "PMIC Arb Version-%d (0x%x)\n", (is_v1 ? 1 : 2),
> + hw_ver);
> +
> + if (is_v1) {
> + pa->ver_ops = &pmic_arb_v1;
> + pa->wr_base = pa->rd_base;
> + } else {
> + u8 chan;
> + u16 ppid;
> + u32 regval;
> +
> + pa->ver_ops = &pmic_arb_v2;
> +
> + pa->ppid_to_chan = devm_kzalloc(&ctrl->dev,
> + PPID_TO_CHAN_TABLE_SZ, GFP_KERNEL);
> + if (!pa->ppid_to_chan) {
> + err = -ENOMEM;
> + goto err_put_ctrl;
> + }
> + /*
> + * PMIC_ARB_REG_CHNL is a table in HW mapping channel to ppid.
> + * ppid_to_chan is an in-memory invert of that table.
> + */
> + for (chan = 0; chan < PMIC_ARB_MAX_CHNL; ++chan) {
> + regval = readl_relaxed(pa->rd_base +
> + PMIC_ARB_REG_CHNL(chan));
> + if (!regval)
> + continue;
> +
> + ppid = (regval >> 8) & 0xFFF;
> + pa->ppid_to_chan[ppid] = chan;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "obsrvr");
> + pa->rd_base = devm_ioremap_resource(&ctrl->dev, res);

...here. Especially because now you have some loose mapping hanging around
for the lifetime of the device. I'd suggest splitting the v1 and v2
probe routines out into their own functions.

Josh


\
 
 \ /
  Last update: 2015-02-18 17:01    [W:1.114 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site