lkml.org 
[lkml]   [2016]   [Jul]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v7 1/4] soc: mediatek: Refine scpsys to support multiple platform
    From
    Date


    On 05/16/2016 11:28 AM, James Liao wrote:
    > Refine scpsys driver common code to support multiple SoC / platform.
    >
    > Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
    > Reviewed-by: Kevin Hilman <khilman@baylibre.com>
    > ---
    > drivers/soc/mediatek/mtk-scpsys.c | 363 +++++++++++++++++++++++---------------
    > 1 file changed, 220 insertions(+), 143 deletions(-)
    >
    > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
    > index 57e781c..5870a24 100644
    > --- a/drivers/soc/mediatek/mtk-scpsys.c
    > +++ b/drivers/soc/mediatek/mtk-scpsys.c
    > @@ -11,17 +11,15 @@
    > * GNU General Public License for more details.
    > */
    > #include <linux/clk.h>
    > -#include <linux/delay.h>
    > +#include <linux/init.h>
    > #include <linux/io.h>
    > -#include <linux/kernel.h>
    > #include <linux/mfd/syscon.h>
    > -#include <linux/init.h>
    > #include <linux/of_device.h>
    > #include <linux/platform_device.h>
    > #include <linux/pm_domain.h>
    > -#include <linux/regmap.h>
    > -#include <linux/soc/mediatek/infracfg.h>
    > #include <linux/regulator/consumer.h>
    > +#include <linux/soc/mediatek/infracfg.h>
    > +
    > #include <dt-bindings/power/mt8173-power.h>
    >
    > #define SPM_VDE_PWR_CON 0x0210
    > @@ -34,6 +32,7 @@
    > #define SPM_MFG_2D_PWR_CON 0x02c0
    > #define SPM_MFG_ASYNC_PWR_CON 0x02c4
    > #define SPM_USB_PWR_CON 0x02cc
    > +
    > #define SPM_PWR_STATUS 0x060c
    > #define SPM_PWR_STATUS_2ND 0x0610
    >
    > @@ -55,12 +54,12 @@
    > #define PWR_STATUS_USB BIT(25)
    >
    > enum clk_id {
    > - MT8173_CLK_NONE,
    > - MT8173_CLK_MM,
    > - MT8173_CLK_MFG,
    > - MT8173_CLK_VENC,
    > - MT8173_CLK_VENC_LT,
    > - MT8173_CLK_MAX,
    > + CLK_NONE,
    > + CLK_MM,
    > + CLK_MFG,
    > + CLK_VENC,
    > + CLK_VENC_LT,
    > + CLK_MAX,
    > };
    >
    > #define MAX_CLKS 2
    > @@ -76,98 +75,6 @@ struct scp_domain_data {
    > bool active_wakeup;
    > };
    >
    > -static const struct scp_domain_data scp_domain_data[] = {
    > - [MT8173_POWER_DOMAIN_VDEC] = {
    > - .name = "vdec",
    > - .sta_mask = PWR_STATUS_VDEC,
    > - .ctl_offs = SPM_VDE_PWR_CON,
    > - .sram_pdn_bits = GENMASK(11, 8),
    > - .sram_pdn_ack_bits = GENMASK(12, 12),
    > - .clk_id = {MT8173_CLK_MM},
    > - },
    > - [MT8173_POWER_DOMAIN_VENC] = {
    > - .name = "venc",
    > - .sta_mask = PWR_STATUS_VENC,
    > - .ctl_offs = SPM_VEN_PWR_CON,
    > - .sram_pdn_bits = GENMASK(11, 8),
    > - .sram_pdn_ack_bits = GENMASK(15, 12),
    > - .clk_id = {MT8173_CLK_MM, MT8173_CLK_VENC},
    > - },
    > - [MT8173_POWER_DOMAIN_ISP] = {
    > - .name = "isp",
    > - .sta_mask = PWR_STATUS_ISP,
    > - .ctl_offs = SPM_ISP_PWR_CON,
    > - .sram_pdn_bits = GENMASK(11, 8),
    > - .sram_pdn_ack_bits = GENMASK(13, 12),
    > - .clk_id = {MT8173_CLK_MM},
    > - },
    > - [MT8173_POWER_DOMAIN_MM] = {
    > - .name = "mm",
    > - .sta_mask = PWR_STATUS_DISP,
    > - .ctl_offs = SPM_DIS_PWR_CON,
    > - .sram_pdn_bits = GENMASK(11, 8),
    > - .sram_pdn_ack_bits = GENMASK(12, 12),
    > - .clk_id = {MT8173_CLK_MM},
    > - .bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
    > - MT8173_TOP_AXI_PROT_EN_MM_M1,
    > - },
    > - [MT8173_POWER_DOMAIN_VENC_LT] = {
    > - .name = "venc_lt",
    > - .sta_mask = PWR_STATUS_VENC_LT,
    > - .ctl_offs = SPM_VEN2_PWR_CON,
    > - .sram_pdn_bits = GENMASK(11, 8),
    > - .sram_pdn_ack_bits = GENMASK(15, 12),
    > - .clk_id = {MT8173_CLK_MM, MT8173_CLK_VENC_LT},
    > - },
    > - [MT8173_POWER_DOMAIN_AUDIO] = {
    > - .name = "audio",
    > - .sta_mask = PWR_STATUS_AUDIO,
    > - .ctl_offs = SPM_AUDIO_PWR_CON,
    > - .sram_pdn_bits = GENMASK(11, 8),
    > - .sram_pdn_ack_bits = GENMASK(15, 12),
    > - .clk_id = {MT8173_CLK_NONE},
    > - },
    > - [MT8173_POWER_DOMAIN_USB] = {
    > - .name = "usb",
    > - .sta_mask = PWR_STATUS_USB,
    > - .ctl_offs = SPM_USB_PWR_CON,
    > - .sram_pdn_bits = GENMASK(11, 8),
    > - .sram_pdn_ack_bits = GENMASK(15, 12),
    > - .clk_id = {MT8173_CLK_NONE},
    > - .active_wakeup = true,
    > - },
    > - [MT8173_POWER_DOMAIN_MFG_ASYNC] = {
    > - .name = "mfg_async",
    > - .sta_mask = PWR_STATUS_MFG_ASYNC,
    > - .ctl_offs = SPM_MFG_ASYNC_PWR_CON,
    > - .sram_pdn_bits = GENMASK(11, 8),
    > - .sram_pdn_ack_bits = 0,
    > - .clk_id = {MT8173_CLK_MFG},
    > - },
    > - [MT8173_POWER_DOMAIN_MFG_2D] = {
    > - .name = "mfg_2d",
    > - .sta_mask = PWR_STATUS_MFG_2D,
    > - .ctl_offs = SPM_MFG_2D_PWR_CON,
    > - .sram_pdn_bits = GENMASK(11, 8),
    > - .sram_pdn_ack_bits = GENMASK(13, 12),
    > - .clk_id = {MT8173_CLK_NONE},
    > - },
    > - [MT8173_POWER_DOMAIN_MFG] = {
    > - .name = "mfg",
    > - .sta_mask = PWR_STATUS_MFG,
    > - .ctl_offs = SPM_MFG_PWR_CON,
    > - .sram_pdn_bits = GENMASK(13, 8),
    > - .sram_pdn_ack_bits = GENMASK(21, 16),
    > - .clk_id = {MT8173_CLK_NONE},
    > - .bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MFG_S |
    > - MT8173_TOP_AXI_PROT_EN_MFG_M0 |
    > - MT8173_TOP_AXI_PROT_EN_MFG_M1 |
    > - MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT,
    > - },
    > -};
    > -
    > -#define NUM_DOMAINS ARRAY_SIZE(scp_domain_data)
    > -
    > struct scp;
    >
    > struct scp_domain {
    > @@ -179,7 +86,7 @@ struct scp_domain {
    > };
    >
    > struct scp {
    > - struct scp_domain domains[NUM_DOMAINS];
    > + struct scp_domain *domains;
    > struct genpd_onecell_data pd_data;
    > struct device *dev;
    > void __iomem *base;
    > @@ -408,57 +315,69 @@ static bool scpsys_active_wakeup(struct device *dev)
    > return scpd->data->active_wakeup;
    > }
    >
    > -static int scpsys_probe(struct platform_device *pdev)
    > +static void init_clks(struct platform_device *pdev, struct clk *clk[CLK_MAX])
    > +{
    > + enum clk_id clk_ids[] = {
    > + CLK_MM,
    > + CLK_MFG,
    > + CLK_VENC,
    > + CLK_VENC_LT
    > + };
    > +
    > + static const char * const clk_names[] = {
    > + "mm",
    > + "mfg",
    > + "venc",
    > + "venc_lt",
    > + };
    > +
    > + int i;
    > +
    > + for (i = 0; i < ARRAY_SIZE(clk_ids); i++)
    > + clk[clk_ids[i]] = devm_clk_get(&pdev->dev, clk_names[i]);

    We can use the global enum clk_id and stat with i = 1, right?

    > +}
    > +
    > +static struct scp *init_scp(struct platform_device *pdev,
    > + const struct scp_domain_data *scp_domain_data, int num)
    > {
    > struct genpd_onecell_data *pd_data;
    > struct resource *res;
    > - int i, j, ret;
    > + int i, j;
    > struct scp *scp;
    > - struct clk *clk[MT8173_CLK_MAX];
    > + struct clk *clk[CLK_MAX];
    >
    > scp = devm_kzalloc(&pdev->dev, sizeof(*scp), GFP_KERNEL);
    > if (!scp)
    > - return -ENOMEM;
    > + return ERR_PTR(-ENOMEM);
    >
    > scp->dev = &pdev->dev;
    >
    > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
    > scp->base = devm_ioremap_resource(&pdev->dev, res);
    > if (IS_ERR(scp->base))
    > - return PTR_ERR(scp->base);
    > -
    > - pd_data = &scp->pd_data;
    > -
    > - pd_data->domains = devm_kzalloc(&pdev->dev,
    > - sizeof(*pd_data->domains) * NUM_DOMAINS, GFP_KERNEL);
    > - if (!pd_data->domains)
    > - return -ENOMEM;
    > -
    > - clk[MT8173_CLK_MM] = devm_clk_get(&pdev->dev, "mm");
    > - if (IS_ERR(clk[MT8173_CLK_MM]))
    > - return PTR_ERR(clk[MT8173_CLK_MM]);
    > -
    > - clk[MT8173_CLK_MFG] = devm_clk_get(&pdev->dev, "mfg");
    > - if (IS_ERR(clk[MT8173_CLK_MFG]))
    > - return PTR_ERR(clk[MT8173_CLK_MFG]);
    > -
    > - clk[MT8173_CLK_VENC] = devm_clk_get(&pdev->dev, "venc");
    > - if (IS_ERR(clk[MT8173_CLK_VENC]))
    > - return PTR_ERR(clk[MT8173_CLK_VENC]);
    > -
    > - clk[MT8173_CLK_VENC_LT] = devm_clk_get(&pdev->dev, "venc_lt");
    > - if (IS_ERR(clk[MT8173_CLK_VENC_LT]))
    > - return PTR_ERR(clk[MT8173_CLK_VENC_LT]);
    > + return ERR_CAST(scp->base);
    >
    > scp->infracfg = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
    > "infracfg");
    > if (IS_ERR(scp->infracfg)) {
    > dev_err(&pdev->dev, "Cannot find infracfg controller: %ld\n",
    > PTR_ERR(scp->infracfg));
    > - return PTR_ERR(scp->infracfg);
    > + return ERR_CAST(scp->infracfg);
    > }
    >
    > - for (i = 0; i < NUM_DOMAINS; i++) {
    > + scp->domains = devm_kzalloc(&pdev->dev,
    > + sizeof(*scp->domains) * num, GFP_KERNEL);
    > + if (!scp->domains)
    > + return ERR_PTR(-ENOMEM);
    > +
    > + pd_data = &scp->pd_data;
    > +
    > + pd_data->domains = devm_kzalloc(&pdev->dev,
    > + sizeof(*pd_data->domains) * num, GFP_KERNEL);
    > + if (!pd_data->domains)
    > + return ERR_PTR(-ENOMEM);
    > +
    > + for (i = 0; i < num; i++) {
    > struct scp_domain *scpd = &scp->domains[i];
    > const struct scp_domain_data *data = &scp_domain_data[i];
    >
    > @@ -467,28 +386,54 @@ static int scpsys_probe(struct platform_device *pdev)
    > if (PTR_ERR(scpd->supply) == -ENODEV)
    > scpd->supply = NULL;
    > else
    > - return PTR_ERR(scpd->supply);
    > + return ERR_CAST(scpd->supply);
    > }
    > }
    >
    > - pd_data->num_domains = NUM_DOMAINS;
    > + pd_data->num_domains = num;
    >
    > - for (i = 0; i < NUM_DOMAINS; i++) {
    > + init_clks(pdev, clk);
    > +
    > + for (i = 0; i < num; i++) {
    > struct scp_domain *scpd = &scp->domains[i];
    > struct generic_pm_domain *genpd = &scpd->genpd;
    > const struct scp_domain_data *data = &scp_domain_data[i];
    >
    > + for (j = 0; j < MAX_CLKS && data->clk_id[j]; j++) {
    > + struct clk *c = clk[data->clk_id[j]];
    > +
    > + if (IS_ERR(c)) {
    > + dev_err(&pdev->dev, "%s: clk unavailable\n",
    > + data->name);
    > + return ERR_CAST(c);
    > + }
    > +
    > + scpd->clk[j] = c;

    Put this in the else branch. Apart from that is there any reason you
    moved the for up in the function? If not, I would prefer not to move it,
    to make it easier to read the diff.

    Apart from that, the patch looks good to me.
    Sorry for the delay in responding.

    Matthias

    \
     
     \ /
      Last update: 2016-07-02 19:01    [W:4.827 / U:0.196 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site