[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5097D93E.10403@ti.com>
Date: Mon, 5 Nov 2012 10:20:30 -0500
From: Murali Karicheri <m-karicheri2@...com>
To: Sekhar Nori <nsekhar@...com>
CC: <mturquette@...aro.org>, <arnd@...db.de>,
<akpm@...ux-foundation.org>, <shawn.guo@...aro.org>,
<rob.herring@...xeda.com>, <linus.walleij@...aro.org>,
<viresh.linux@...il.com>, <linux-kernel@...r.kernel.org>,
<khilman@...com>, <linux@....linux.org.uk>, <sshtylyov@...sta.com>,
<davinci-linux-open-source@...ux.davincidsp.com>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-keystone@...t.ti.com>
Subject: Re: [PATCH v3 03/11] clk: davinci - common clk utilities to init
clk driver
On 11/03/2012 08:35 AM, Sekhar Nori wrote:
> On 10/25/2012 9:41 PM, Murali Karicheri wrote:
>> This is the common clk driver initialization functions for DaVinci
>> SoCs and other SoCs that uses similar hardware architecture.
>> clock.h also defines struct types for clock definitions in a SoC
>> and clock data type for configuring clk-mux. The initialization
>> functions are used by clock initialization code in a specific
>> platform/SoC.
>>
>> Signed-off-by: Murali Karicheri <m-karicheri2@...com>
>> ---
>> drivers/clk/davinci/clock.c | 112 +++++++++++++++++++++++++++++++++++++++++++
>> drivers/clk/davinci/clock.h | 80 +++++++++++++++++++++++++++++++
>> 2 files changed, 192 insertions(+)
>> create mode 100644 drivers/clk/davinci/clock.c
>> create mode 100644 drivers/clk/davinci/clock.h
>>
>> diff --git a/drivers/clk/davinci/clock.c b/drivers/clk/davinci/clock.c
>> new file mode 100644
>> index 0000000..ad02149
>> --- /dev/null
>> +++ b/drivers/clk/davinci/clock.c
>> @@ -0,0 +1,112 @@
>> +/*
>> + * clock.c - davinci clock initialization functions for various clocks
>> + *
>> + * Copyright (C) 2006-2012 Texas Instruments.
>> + * Copyright (C) 2008-2009 Deep Root Systems, LLC
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +#include <linux/init.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/io.h>
>> +#include <linux/slab.h>
>> +
>> +#include "clk-pll.h"
>> +#include "clk-psc.h"
>> +#include "clk-div.h"
>> +#include "clock.h"
>> +
>> +static DEFINE_SPINLOCK(_lock);
>> +
>> +#ifdef CONFIG_CLK_DAVINCI_PLL
>> +struct clk *davinci_pll_clk(const char *name, const char *parent,
>> + u32 phys_pllm, u32 phys_prediv, u32 phys_postdiv,
>> + struct clk_pll_data *pll_data)
>> +{
>> + struct clk *clkp = NULL;
>> +
>> + pll_data->reg_pllm = ioremap(phys_pllm, 4);
>> + if (WARN_ON(!pll_data->reg_pllm))
>> + return clkp;
> I would prefer ERR_PTR(-ENOMEM) here. Same comment applies to other
> instances elsewhere in the patch.
>
>> diff --git a/drivers/clk/davinci/clock.h b/drivers/clk/davinci/clock.h
>> new file mode 100644
>> index 0000000..73204b8
>> --- /dev/null
>> +++ b/drivers/clk/davinci/clock.h
>> @@ -0,0 +1,80 @@
>> +/*
>> + * TI DaVinci Clock definitions - Contains Macros and Types used for
>> + * defining various clocks on a DaVinci SoC
>> + *
>> + * Copyright (C) 2012 Texas Instruments
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +#ifndef __DAVINCI_CLOCK_H
>> +#define __DAVINCI_CLOCK_H
>> +
>> +#include <linux/types.h>
>> +
>> +/* general flags: */
>> +#define ALWAYS_ENABLED BIT(0)
> This is not used in this patch. Can you add the define along with its
> usage so it is immediately clear why you need it?
This is used on the next patch as this adds a bunch of utilities or
types for the platform specific clock code. Do you want to combine this
patch with 05/11? It will become a bigger patch then? Is that fine.
IMO, this is a logical group that can be a standalone patch than
combining with 05/11.
>> +/**
>> + * struct davinci_clk - struct for defining DaVinci clocks for a SoC.
>> + *
>> + * @name: name of the clock
>> + * @parent: name of parent clock
>> + * @flags: General flags for all drivers used by platform clock init code
>> + * @data: data specific to a clock used by the driver
>> + * @dev_id: dev_id used to look up this clock. If this is NULL
>> + * clock name is used for lookup.
>> + */
>> +struct davinci_clk {
>> + const char *name;
>> + const char *parent;
>> + u32 flags;
>> + void *data;
>> + char *dev_id;
> Similarly dont see this being used as well.
For this and previous one, check lpsc_clk() macro usage. dev_id is used
where a specific device id is used to look up
the clk as is done for davinci_emac.1, davinci_mmc.0 etc.
>
> Thanks,
> Sekhar
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists