* [PATCH RFT 3/3] ARM: tegra: dts: seaboard: enable keyboard
From: Russell King - ARM Linux @ 2013-01-12 10:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F13C34.3020808@nvidia.com>
On Sat, Jan 12, 2013 at 04:04:28PM +0530, Laxman Dewangan wrote:
> Going through clock driver, the clk_get() should return error pointer or
> valid pointer, atleast not NULL.
No. clk_get() will return whatever value the 'clk' pointer found in
the clk_lookup structure contains. Or it will return an error pointer.
This topic has had enough discussion in the past that I'll say: go and
look up in the archives emails from me on this subject. I've been very
clear over the years about this.
> Probably our clock driver is returning to NULL and hence issue is
> becasue whole world, it check for IS_ERR or !IS_ERR() and null clk
> pointer treated as !IS_ERR().
Probably not. As far as _drivers_ go, either the clk cookie is an error
(IS_ERR() is true) _OR_ it is valid to be passed back into the clock API.
You will find statements from me in the past to that: whatever clk_get()
returns which is _not_ an error must be accepted by the rest of the clk
API.
Drivers never dereference the returned clk, so why should they care about
anything other than "is it an error" given by IS_ERR(). (notice the lack
of question mark, that's not a question to be answered.)
^ permalink raw reply
* [PATCH RFT 3/3] ARM: tegra: dts: seaboard: enable keyboard
From: Laxman Dewangan @ 2013-01-12 10:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F0A2A8.3080401@wwwdotorg.org>
On Saturday 12 January 2013 05:09 AM, Stephen Warren wrote:
> On 01/11/2013 04:36 PM, Russell King - ARM Linux wrote:
>> On Fri, Jan 11, 2013 at 04:27:07PM -0700, Stephen Warren wrote:
>>> On 01/11/2013 04:24 PM, Russell King - ARM Linux wrote:
>>>> On Fri, Jan 11, 2013 at 04:09:59PM -0700, Stephen Warren wrote:
>>>>> On 01/11/2013 06:33 AM, Laxman Dewangan wrote:
>>>>>> kbc->clk = clk_get(&pdev->dev, NULL);
>>>>>> if (IS_ERR(kbc->clk)) {
>>>>>> dev_err(&pdev->dev, "failed to get keyboard clock\n");
>>>>>> err = PTR_ERR(kbc->clk);
>>>>>> goto err_iounmap;
>>>>>> }
>>>>> Should that check be if (!kbc-clk) instead? Or does the common clock
>>>>> framework require if (IS_ERR_OR_NULL(kbc->clk)); hopefully not since
>>>>> IS_ERR_OR_NULL shouldn't be used any more.
>>>> /**
>>>> * clk_get - lookup and obtain a reference to a clock producer.
>>>> * @dev: device for clock "consumer"
>>>> * @id: clock consumer ID
>>>> *
>>>> * Returns a struct clk corresponding to the clock producer, or
>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>> * valid IS_ERR() condition containing errno. ...
>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>
>>>> Or, put another way:
>>>>
>>>> If (!IS_ERR(clk))
>>>> The_Clock_Is_Valid();
>>>> Else
>>>> The_Clock_Is_Invalid();
>>>> The_Error = PTR_ERR(clk);
>>> OK, but that doesn't appear to be what happened in practice.
>> It's what I've been saying each time I see an abuse, the problem is
>> people don't care to read the documentation provided let alone
>> understand the interfaces.
>>
>> That's precisely why IS_ERR_OR_NULL() is to be removed. One less
>> thing for someone to throw a dart at as a selection method to use.
>> Or maybe roll a dice. Or whatever way they do seem to choose.
>> (Whatever that is, it's not based on any sound engineering practice
>> I can make out.)
> For the record, I did mention that IS_ERR_OR_NULL() should not be the
> solution here. And the point of my email to Laxman was that he should go
> figure out the answer to my question, which would entail reading the
> documentation/code/...
Going through clock driver, the clk_get() should return error pointer or
valid pointer, atleast not NULL.
Probably our clock driver is returning to NULL and hence issue is
becasue whole world, it check for IS_ERR or !IS_ERR() and null clk
pointer treated as !IS_ERR().
Stephen,
Which is the tree on which Prashant changes are applied. I can debug
from that tree.
^ permalink raw reply
* [PATCH RFT 3/3] ARM: tegra: dts: seaboard: enable keyboard
From: Laxman Dewangan @ 2013-01-12 10:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50F09BC7.90607@wwwdotorg.org>
On Saturday 12 January 2013 04:39 AM, Stephen Warren wrote:
> On 01/11/2013 06:33 AM, Laxman Dewangan wrote:
>> Enable tegra based keyboard controller and populate the key matrix for
>> seaboard. The key matrix was originally on driver code which is removed
>> to have clean driver. The key mapping is now passed through dts file.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>> ---
>> Requesting for testing on seaboard.
>> I generated this patch as Stephen suggested to have one patch for dt file
>> entry for keys on seaboard.
Thanks for testing.
> To some extent this works; at least this patch series is probably almost
> OK, but there are a bunch of issues in the KBC driver obviously:
>
> This series won't work on its own with the current Tegra for-next
> content, because:
>
> a) The clock driver doesn't provide any KBC clock for Tegra20, either in
> the old Tegra clock driver, or the first conversion to the common clock
> framework, nor the most recent complete conversion to the common clock
> framework. Was the driver tested at all on the upstream kernel? I
> suppose it's just possible it was tested on Tegra30, although see below...
I have cardhu and Ventana and unfortunately both does not support the
key pad matrix. Both are on gpio keys.
I tested this patch series (along with recent kbc patches) on t114
platform Dalmore, it is 3x3 matrix.
Yes I made a hack on the board-dt-* file for getting proper clock. I did
not send similar change as this will not be very much used when we will
we in CCF.
> This brings up the point that the clk_get() call in the KBC driver is
> probably wrong. At least with Prashant's latest common clock framework
> patches applied, clk_get() returns NULL, not an error pointer for a
> non-existent clock. As such, the following driver code succeeds:
>
>> kbc->clk = clk_get(&pdev->dev, NULL);
>> if (IS_ERR(kbc->clk)) {
>> dev_err(&pdev->dev, "failed to get keyboard clock\n");
>> err = PTR_ERR(kbc->clk);
>> goto err_iounmap;
>> }
> Should that check be if (!kbc-clk) instead? Or does the common clock
> framework require if (IS_ERR_OR_NULL(kbc->clk)); hopefully not since
> IS_ERR_OR_NULL shouldn't be used any more.
If clk_get() failed then it should be return ptr_err. if not then it
need to change almost all driver i.e. i2c/spi.
If clk_get(0 returns NULL then should be part of the clock CCF changes
series to change here also.
> This then causes the later call to tegra_periph_reset_{de,}assert() to
> crash since it generates a bogus pointer from the NULL pointer and
> dereferences it. I assume similar things happen before Prashant's latest
> clock patches.
No, Currently clk_get() returns the PTR_ERR and probe fail. This is well
tested till now. Something changed with CCF.
>
> Oh, and the KBC clock is marked TEGRA_PERIPH_NO_RESET for Tegra30, and
> also for Tegra20 in the ChromeOS kernel; why is the driver trying to
> assert reset on a clock that doesn't support it?
This is something our kbc controller reset and clock design.
KBC controller is on Always Power ON domain so that it can work even
when system is in sleep.
The KBC clock is enabled through two places, PMC control register and
CAR register set.
KBC controller is only reset through PMC control register.
This is not well implemented either on our downstream or in mainline.
Sometime back I tried to implement it in downstream but was having lots
of comment and not able to complete this. Possibly we will talk
internally that how can we implement this.
>
> I hacked around this by adding the following to clk-tegra20.c on top of
> Prashant's latest patches:
>
>> /* kbc */
>> clk = tegra_clk_periph_gate("kbc", "clk_32k", TEGRA_PERIPH_NO_RESET |
>> TEGRA_PERIPH_ON_APB, clk_base, 0,
>> 36, &periph_h_regs, periph_clk_enb_refcnt);
>> clk_register_clkdev(clk, NULL, "tegra-kbc");
>> clks[kbc] = clk;
>>
> b) There is no AUXDATA in the board file, nor clocks properties in the
> .dts files, in this series to actually hook the KBC driver up to be able
> to get the correct clock. This leads me to suspect the driver was never
> tested upstream on either Tegra20 /or/ Tegra30.
I have my local change for the AUXDATA board file which I did not sent.
I think CCF does not require and this is the reason. I can sent this if
it needed.
> I hacked around this by applying the following:
Yes, hack is required here. Either hack in the board file for auxdata or
in dts file for clock entry when you have ccf ready.
> I guess I can fold that fix into this series when I apply it, after
> Prashant's clock patches are merged.
>
> Once I hacked around the above issues, the driver doesn't work very well
> at all. There is a *long* delay between when I press a key and when it
> shows up (e.g. echo'd to the HDMI console). When I release the key the
> same keypress is generated twice. Or perhaps it's a repeat, but since
> it's *always* 2 extra keypresses and I doubt I always hold my finger on
> the key the exact same amount of time, I think it's key release that
> does this not repeat. I assume this would repro on any board, so you
> won't need Seaboard to debug it. Harmony is able to read the keyboard
> using the KBC.
>
> Even with the above, I was able to validate that the keymap in the
> Seaboard .dts file looks reasonable.
>
> However, please fix the KBC driver...
I did not see any issue with the 3x3 matrix. I think all these about is
to tuning debaunce and other parameter also. Another thing is that to
make sure that all pinmuxes are properly configured.
However, again I will try to enable kbc on harmony and run it.
^ permalink raw reply
* [PATCH v2] clk: max77686: Remove unnecessary NULL checking for container_of()
From: Axel Lin @ 2013-01-12 10:01 UTC (permalink / raw)
To: linux-arm-kernel
container_of() never returns NULL, thus remove the NULL checking for it.
Also rename get_max77686_clk() to to_max77686_clk() for better readability.
Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
v2: remove unrelated change for ~max77686->mask
drivers/clk/clk-max77686.c | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)
diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
index 8944214..90bf59c 100644
--- a/drivers/clk/clk-max77686.c
+++ b/drivers/clk/clk-max77686.c
@@ -44,33 +44,23 @@ struct max77686_clk {
struct clk_lookup *lookup;
};
-static struct max77686_clk *get_max77686_clk(struct clk_hw *hw)
+static struct max77686_clk *to_max77686_clk(struct clk_hw *hw)
{
return container_of(hw, struct max77686_clk, hw);
}
static int max77686_clk_prepare(struct clk_hw *hw)
{
- struct max77686_clk *max77686;
- int ret;
-
- max77686 = get_max77686_clk(hw);
- if (!max77686)
- return -ENOMEM;
-
- ret = regmap_update_bits(max77686->iodev->regmap,
- MAX77686_REG_32KHZ, max77686->mask, max77686->mask);
+ struct max77686_clk *max77686 = to_max77686_clk(hw);
- return ret;
+ return regmap_update_bits(max77686->iodev->regmap,
+ MAX77686_REG_32KHZ, max77686->mask,
+ max77686->mask);
}
static void max77686_clk_unprepare(struct clk_hw *hw)
{
- struct max77686_clk *max77686;
-
- max77686 = get_max77686_clk(hw);
- if (!max77686)
- return;
+ struct max77686_clk *max77686 = to_max77686_clk(hw);
regmap_update_bits(max77686->iodev->regmap,
MAX77686_REG_32KHZ, max77686->mask, ~max77686->mask);
@@ -78,14 +68,10 @@ static void max77686_clk_unprepare(struct clk_hw *hw)
static int max77686_clk_is_enabled(struct clk_hw *hw)
{
- struct max77686_clk *max77686;
+ struct max77686_clk *max77686 = to_max77686_clk(hw);
int ret;
u32 val;
- max77686 = get_max77686_clk(hw);
- if (!max77686)
- return -ENOMEM;
-
ret = regmap_read(max77686->iodev->regmap,
MAX77686_REG_32KHZ, &val);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v5 6/9] ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
From: Russell King - ARM Linux @ 2013-01-12 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAK=WgbZVayi-USPCwB3KSbzRQ_oxatznt4d26A8_GpWmay2daA@mail.gmail.com>
On Fri, Jan 11, 2013 at 02:26:19PM +0200, Ohad Ben-Cohen wrote:
> > +static int davinci_rproc_start(struct rproc *rproc)
> > +{
> > + struct platform_device *pdev = to_platform_device(rproc->dev.parent);
> > + struct device *dev = rproc->dev.parent;
> > + struct davinci_rproc *drproc = rproc->priv;
> > + struct clk *dsp_clk;
> > + struct resource *r;
> > + unsigned long host1cfg_physaddr;
> > + unsigned int host1cfg_offset;
> > + int ret;
> > +
> > + remoteprocdev = pdev;
> > +
> > + /* hw requires the start (boot) address be on 1KB boundary */
> > + if (rproc->bootaddr & 0x3ff) {
> > + dev_err(dev, "invalid boot address: must be aligned to 1KB\n");
> > +
> > + return -EINVAL;
> > + }
> > +
> > + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (IS_ERR_OR_NULL(r)) {
No, this is buggy. Go and look up to see what the return ranges are
for this function.
> > + dev_err(dev, "platform_get_resource() error: %ld\n",
> > + PTR_ERR(r));
> > +
> > + return PTR_ERR(r);
Which results in this being a bug.
> > + }
> > + host1cfg_physaddr = (unsigned long)r->start;
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0) {
> > + dev_err(dev, "platform_get_irq(pdev, 0) error: %d\n", irq);
> > +
> > + return irq;
> > + }
> > +
> > + irq_data = irq_get_irq_data(irq);
> > + if (IS_ERR_OR_NULL(irq_data)) {
Again, bug.
> > + dev_err(dev, "irq_get_irq_data(%d) error: %ld\n",
> > + irq, PTR_ERR(irq_data));
> > +
> > + return PTR_ERR(irq_data);
Which results in this being a bug.
> > + }
> > + ack_fxn = irq_data->chip->irq_ack;
> > +
> > + ret = request_threaded_irq(irq, davinci_rproc_callback, handle_event,
> > + 0, "davinci-remoteproc", drproc);
> > + if (ret) {
> > + dev_err(dev, "request_threaded_irq error: %d\n", ret);
> > +
> > + return ret;
> > + }
> > +
> > + syscfg0_base = ioremap(host1cfg_physaddr & PAGE_MASK, SZ_4K);
> > + host1cfg_offset = offset_in_page(host1cfg_physaddr);
> > + writel(rproc->bootaddr, syscfg0_base + host1cfg_offset);
> > +
> > + dsp_clk = clk_get(dev, NULL);
> > + if (IS_ERR_OR_NULL(dsp_clk)) {
And another bug.
> > + dev_err(dev, "clk_get error: %ld\n", PTR_ERR(dsp_clk));
> > + ret = PTR_ERR(dsp_clk);
And again, results in this being a bug.
> > + goto fail;
> > + }
...
> > + ret = rproc_add(rproc);
> > + if (ret)
> > + goto free_rproc;
> > +
> > + /*
> > + * rproc_add() can end up enabling the DSP's clk with the DSP
> > + * *not* in reset, but davinci_rproc_start() needs the DSP to be
> > + * held in reset at the time it is called.
> > + */
> > + dsp_clk = clk_get(rproc->dev.parent, NULL);
> > + davinci_clk_reset_assert(dsp_clk);
> > + clk_put(dsp_clk);
BUG: what if clk_get() fails here?
^ permalink raw reply
* [PATCH 1/2] ARM: sunxi: gpio: Add Allwinner SoCs GPIO drivers
From: Maxime Ripard @ 2013-01-12 9:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkda0yhVvCW9edWBi3XusfS4NaiWqKsoCfLpT9+X5iO-GXg@mail.gmail.com>
Hi Linus,
On 10/01/2013 12:06, Linus Walleij wrote:
> On Fri, Jan 4, 2013 at 5:45 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> +static int __devinit
>> +sunxi_pinctrl_register_gpio_ranges(struct sunxi_pinctrl *pctl)
>> +{
>> + int id = 0, base = 0, npins = 1, i, prev_pin = -1;
>> + struct pinctrl_gpio_range *range;
>> +
>> + for (i = 0; i < pctl->desc->npins; i++) {
>> + const struct sunxi_desc_pin *pin = pctl->desc->pins + i;
>> + unsigned pin_num = pin->pin.number;
>> +
>> + if (prev_pin < 0) {
>> + prev_pin = pin_num;
>> + base = pin_num;
>> + } else if (prev_pin + 1 != pin_num) {
>> + range = devm_kzalloc(pctl->dev,
>> + sizeof(*range),
>> + GFP_KERNEL);
>> + if (!range)
>> + return -ENOMEM;
>> +
>> + range->name = "sunxi";
>> + range->id = id;
>> + range->base = base;
>> + range->pin_base = base;
>> + range->npins = npins;
>> +
>> + pinctrl_add_gpio_range(pctl->pctl_dev, range);
>> +
>> + id++;
>> + npins = 1;
>> + prev_pin = pin_num;
>> + base = prev_pin;
>> + } else {
>> + prev_pin++;
>> + npins++;
>> + }
>> + }
>> +
>> + range = devm_kzalloc(pctl->dev, sizeof(*range),
>> + GFP_KERNEL);
>> + if (!range)
>> + return -ENOMEM;
>> +
>> + range->name = "sunxi";
>> + range->id = id;
>> + range->base = base;
>> + range->pin_base = base;
>> + range->npins = npins;
>> +
>> + pinctrl_add_gpio_range(pctl->pctl_dev, range);
>> +
>> + return 0;
>> +}
>
> Really hairy way to add ranges right?
Yes... I didn't find any best way to do so. It was either that or
duplicate the already existing informations we had about the pins ranges
into a ranges array, which is pretty error-prone.
I guess I could add some comments though.
> Registering ranges from the pinctrl side is deprecated and discouraged.
>
> Instead register the ranges from the GPIO driver.
>
> Use gpiochip_add_pin_range() from the GPIO driver.
>
> An example is provided in
> drivers/pinctrl/pinctrl-coh901.c
Ok, will do. But we need to find a way to share the pins arrays between
the pinctrl and gpio drivers then.
Maybe add a pinctrl-sunxi-pins.h file? or merge the pinctrl and gpio
drivers?
I'm kind of reluctant to merging the drivers into one single messy file,
but if that's the way to go, fine.
>
> After you have done this, you can probably get rid of this as well:
>
> +static int __init sunxi_gpio_init(void)
> +{
> + return platform_driver_register(&sunxi_gpio_driver);
> +}
> +postcore_initcall(sunxi_gpio_init);
>
> This will become a simple module_init() if you are handling
> deferred probe correctly.
Ok. Thanks for your review!
Maxime
--
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH 0/2] Add GPIO driver for Allwinner SoCs
From: Maxime Ripard @ 2013-01-12 8:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdZQAim4_3E6RG=d4pJi80B6W-xzW7n=Od86VJNcPO4FVw@mail.gmail.com>
On 10/01/2013 12:13, Linus Walleij wrote:
> On Sat, Jan 5, 2013 at 4:35 PM, Olof Johansson <olof@lixom.net> wrote:
>
>> I haven't looked closely at the patch yet, but this is a case where it
>> makes sense to merge the gpio/pinctrl patch through Linus/Grant's
>> tree, and merge the dtsi update through arm-soc -- while you need
>> both, nothing is broken by having just one or the other, so taking the
>> separate merge paths are fine.
>>
>> Linus, Grant, sound ok to you?
>
> Sure. But I have requested a few changes to the patch set...
And we need to merge the pinctrl patches first also.
Maxime
--
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH 15/16] ARM: vexpress/dcscb: handle platform coherency exit/setup and CCI
From: Santosh Shilimkar @ 2013-01-12 7:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301111425020.6300@xanadu.home>
On Saturday 12 January 2013 12:58 AM, Nicolas Pitre wrote:
> On Fri, 11 Jan 2013, Santosh Shilimkar wrote:
>
>> On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
>>> From: Dave Martin <dave.martin@linaro.org>
>>>
>>> + /*
>>> + * Flush the local CPU cache.
>>> + *
>>> + * A15/A7 can hit in the cache with SCTLR.C=0, so we don't need
>>> + * a preliminary flush here for those CPUs. At least, that's
>>> + * the theory -- without the extra flush, Linux explodes on
>>> + * RTSM (maybe not needed anymore, to be investigated).
>>> + */
>> This is expected if the entire code is not in one stack frame and the
>> additional flush is needed to avoid possible stack corruption. This
>> issue has been discussed in past on the list.
>
> I missed that. Do you have a reference or pointer handy?
>
> What is strange is that this is 100% reproducible on RTSM while this
> apparently is not an issue on real hardware so far.
>
I tried searching archives and realized the discussion was in private
email thread. There are some bits and pieces on list but not all the
information.
The main issue RMK pointed out is- An additional L1 flush needed
to avoid the effective change of view of memory when the C bit is
turned off, and the cache is no longer searched for local CPU accesses.
In your case dcscb_power_down() has updated the stack which can be hit
in cache line and hence cache is dirty now. Then cpu_proc_fin() clears
the C-bit and hence for sub sequent calls the L1 cache won't be
searched. You then call flush_cache_all() which again updates the
stack but avoids searching the L1 cache. So it overwrites previous
saved stack frame. This seems to be an issue in your case as well.
Regards,
Santosh
^ permalink raw reply
* [PATCH 13/16] drivers: misc: add ARM CCI support
From: Santosh Shilimkar @ 2013-01-12 6:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301111414390.6300@xanadu.home>
On Saturday 12 January 2013 12:52 AM, Nicolas Pitre wrote:
> On Fri, 11 Jan 2013, Santosh Shilimkar wrote:
>
>> On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
>>> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>
>>> On ARM multi-cluster systems coherency between cores running on
>>> different clusters is managed by the cache-coherent interconnect (CCI).
>>> It allows broadcasting of TLB invalidates and memory barriers and it
>>> guarantees cache coherency at system level.
>>>
>>> This patch enables the basic infrastructure required in Linux to
>>> handle and programme the CCI component. The first implementation is
>>> based on a platform device, its relative DT compatible property and
>>> a simple programming interface.
>>>
>>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
>>> ---
>>> drivers/misc/Kconfig | 3 ++
>>> drivers/misc/Makefile | 1 +
>>> drivers/misc/arm-cci.c | 107
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/arm-cci.h | 30 ++++++++++++++
>> How about 'drivers/bus/' considering CCI is an interconnect bus (though
>> for coherency)
>
> Yes, I like that better.
>
Great.
>>> 4 files changed, 141 insertions(+)
>>> create mode 100644 drivers/misc/arm-cci.c
>>> create mode 100644 include/linux/arm-cci.h
>>>
>>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>>> index b151b7c1bd..30d5be1ad2 100644
>>> --- a/drivers/misc/Kconfig
>>> +++ b/drivers/misc/Kconfig
>>> @@ -499,6 +499,9 @@ config USB_SWITCH_FSA9480
>>> stereo and mono audio, video, microphone and UART data to use
>>> a common connector port.
>>>
>>> +config ARM_CCI
>> You might want add depends on ARM big.LITTTLE otherwise it will
>> break build for other arch's with random configurations.
>
> As far as this patch goes, this is buildable on other architectures too.
> The next patch changes that though.
>
Thanks.
>> [..]
>>
>>> diff --git a/drivers/misc/arm-cci.c b/drivers/misc/arm-cci.c
>>> new file mode 100644
>>> index 0000000000..f329c43099
>>> --- /dev/null
>>> +++ b/drivers/misc/arm-cci.c
>>> @@ -0,0 +1,107 @@
>>> +/*
>>> + * CCI support
>>> + *
>>> + * Copyright (C) 2012 ARM Ltd.
>>> + * Author: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#include <linux/device.h>
>>> +#include <linux/io.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/arm-cci.h>
>>> +
>>> +#define CCI400_EAG_OFFSET 0x4000
>>> +#define CCI400_KF_OFFSET 0x5000
>>> +
>>> +#define DRIVER_NAME "CCI"
>>> +struct cci_drvdata {
>>> + void __iomem *baseaddr;
>>> + spinlock_t lock;
>>> +};
>>> +
>>> +static struct cci_drvdata *info;
>>> +
>>> +void disable_cci(int cluster)
>>> +{
>>> + u32 cci_reg = cluster ? CCI400_KF_OFFSET : CCI400_EAG_OFFSET;
>>> + writel_relaxed(0x0, info->baseaddr + cci_reg);
>>> +
>>> + while (readl_relaxed(info->baseaddr + 0xc) & 0x1)
>>> + ;
>>> +}
>>> +EXPORT_SYMBOL_GPL(disable_cci);
>>> +
>> Is more functionality going to be added for CCI driver. Having this
>> much of driver code for just a disable_cci() functions seems like
>> overkill.
>
> Yes. More code will appear here to provide pmu functionalities, etc.
>
Good to know.
Regards,
Santosh
^ permalink raw reply
* [PATCH 10/16] ARM: vexpress: introduce DCSCB support
From: Santosh Shilimkar @ 2013-01-12 6:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301111408330.6300@xanadu.home>
On Saturday 12 January 2013 12:43 AM, Nicolas Pitre wrote:
> On Fri, 11 Jan 2013, Santosh Shilimkar wrote:
>
>> On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
>>> This adds basic CPU and cluster reset controls on RTSM for the
>>> A15x4-A7x4 model configuration using the Dual Cluster System
>>> Configuration Block (DCSCB).
>>>
>>> The cache coherency interconnect (CCI) is not handled yet.
>>>
>>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
>>> ---
>>> arch/arm/mach-vexpress/Kconfig | 8 ++
>>> arch/arm/mach-vexpress/Makefile | 1 +
>>> arch/arm/mach-vexpress/dcscb.c | 160
>>> ++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 169 insertions(+)
>>> create mode 100644 arch/arm/mach-vexpress/dcscb.c
>>>
[..]
>>> diff --git a/arch/arm/mach-vexpress/dcscb.c b/arch/arm/mach-vexpress/dcscb.c
>>> new file mode 100644
>>> index 0000000000..cccd943cd4
>>> --- /dev/null
>>> +++ b/arch/arm/mach-vexpress/dcscb.c
[..]
>>> +static void dcscb_power_down(void)
>>> +{
>>> + unsigned int mpidr, cpu, cluster, rst_hold, cpumask, last_man;
>>> +
>>> + asm ("mrc p15, 0, %0, c0, c0, 5" : "=r" (mpidr));
>>> + cpu = mpidr & 0xff;
>>> + cluster = (mpidr >> 8) & 0xff;
>>> + cpumask = (1 << cpu);
>>> +
>>> + pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
>>> + BUG_ON(cpu >= 4 || cluster >= 2);
>>> +
>>> + arch_spin_lock(&dcscb_lock);
>>> + rst_hold = readl_relaxed(dcscb_base + RST_HOLD0 + cluster * 4);
>>> + rst_hold |= cpumask;
>>> + if (((rst_hold | (rst_hold >> 4)) & 0xf) == 0xf)
>>> + rst_hold |= (1 << 8);
>>> + writel(rst_hold, dcscb_base + RST_HOLD0 + cluster * 4);
>>> + arch_spin_unlock(&dcscb_lock);
>>> + last_man = (rst_hold & (1 << 8));
>>> +
>>> + /*
>>> + * Now let's clean our L1 cache and shut ourself down.
>>> + * If we're the last CPU in this cluster then clean L2 too.
>>> + */
>>> +
>> Do you wanted to have C bit clear code here ?
>
> cpu_proc_fin() does it.
>
Yep. I noticed that in next patch when read the comment.
>>> + /*
>>> + * A15/A7 can hit in the cache with SCTLR.C=0, so we don't need
>>> + * a preliminary flush here for those CPUs. At least, that's
>>> + * the theory -- without the extra flush, Linux explodes on
>>> + * RTSM (maybe not needed anymore, to be investigated)..
>>> + */
>>> + flush_cache_louis();
>>> + cpu_proc_fin();
>>> +
>>> + if (!last_man) {
>>> + flush_cache_louis();
>>> + } else {
>>> + flush_cache_all();
>>> + outer_flush_all();
>>> + }
>>> +
>>> + /* Disable local coherency by clearing the ACTLR "SMP" bit: */
>>> + asm volatile (
>>> + "mrc p15, 0, ip, c1, c0, 1 \n\t"
>>> + "bic ip, ip, #(1 << 6) @ clear SMP bit \n\t"
>>> + "mcr p15, 0, ip, c1, c0, 1"
>>> + : : : "ip" );
>>> +
>>> + /* Now we are prepared for power-down, do it: */
>> You need dsb here, right ?
>
> Probably. However this code is being refactored significantly with
> subsequent patches. This intermediate step was kept not to introduce
> too many concepts at once.
>
Yes. I do see updates in subsequent patch.
Regards
Santosh
^ permalink raw reply
* [PATCH 08/16] ARM: bL_platsmp.c: make sure the GIC interface of a dying CPU is disabled
From: Santosh Shilimkar @ 2013-01-12 6:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301111405590.6300@xanadu.home>
On Saturday 12 January 2013 12:37 AM, Nicolas Pitre wrote:
> On Fri, 11 Jan 2013, Santosh Shilimkar wrote:
>
>> On Thursday 10 January 2013 05:50 AM, Nicolas Pitre wrote:
>>> Otherwise there might be some interrupts or IPIs becoming pending and the
>>> CPU will not enter low power mode when doing a WFI. The effect of this
>>> is a CPU that loops back into the kernel, go through the first man
>>> election, signals itself as alive, and prevent the cluster from being
>>> shut down.
>>>
>>> This could benefit from a better solution.
>>>
>>> Signed-off-by: Nicolas Pitre <nico@linaro.org>
>>> ---
>>> arch/arm/common/bL_platsmp.c | 1 +
>>> arch/arm/common/gic.c | 6 ++++++
>>> arch/arm/include/asm/hardware/gic.h | 2 ++
>>> 3 files changed, 9 insertions(+)
>>>
>>> diff --git a/arch/arm/common/bL_platsmp.c b/arch/arm/common/bL_platsmp.c
>>> index 0ae44123bf..6a3b251b97 100644
>>> --- a/arch/arm/common/bL_platsmp.c
>>> +++ b/arch/arm/common/bL_platsmp.c
>>> @@ -68,6 +68,7 @@ static void __ref bL_cpu_die(unsigned int cpu)
>>> pcpu = mpidr & 0xff;
>>> pcluster = (mpidr >> 8) & 0xff;
>>> bL_set_entry_vector(pcpu, pcluster, NULL);
>>> + gic_cpu_if_down();
>>
>> So for a case where CPU still don't power down for some reason even
>> after CPU interface is disabled, can not listen to and SGI or PPI.
>> Not sure if this happens on big.LITTLE but i have seen one such issue
>> on Cortex-A9 based SOC.
>
> Here the problem was the reverse i.e. a CPU wouldn't go down because
> some pending SGIs prevented that.
>
I understood that part. What I was saying is, with CPU IF disabled and
if CPU doesn't enter into the intended low power state and if the wakeup
mechanism on that CPU is SGI/SPI, CPU may never wakeup and can lead to
dead lock. I have seen this scenario on OMAP especially in CPUidle path.
It may not be relevant for switcher considering, you almost force CPU to
enter to low power state :-)
Regards,
Santosh
^ permalink raw reply
* [PATCH 3/4] ARM: imx: restore uncompress.h to a common place
From: Dirk Behme @ 2013-01-12 6:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1355319945-3199-4-git-send-email-shawn.guo@linaro.org>
On 12.12.2012 14:45, Shawn Guo wrote:
> Restore arch/arm/plat-mxc/include/mach/uncompress.h removed by commit
> 3995eb8 (ARM: imx: merge plat-mxc into mach-imx) to a common place as
> arch/arm/include/debug/uncompress/imx.h.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
...
> +++ b/arch/arm/include/debug/uncompress/imx.h
...
> + * arch/arm/plat-mxc/include/mach/uncompress.h
In case you touch these patches again, you might check if the above
path and file name in the file header should be adapted to the new
file location, too.
Best regards
Dirk
^ permalink raw reply
* Question:add Iirq mask method in irq disable call.
From: steve.zhan @ 2013-01-12 4:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.LFD.2.02.1301111652100.7475@ionos>
2013/1/11 Thomas Gleixner <tglx@linutronix.de>:
> On Fri, 11 Jan 2013, steve.zhan wrote:
>> Resend Email because MUTT email send error.
>>
>> 2013/1/11 Steve zhan <zhanzhenbo@gmail.com>:
>> > All:
>> > I have find that arm gic driver have not register irq_disable
>> > method, as below:
>> > static struct irq_chip gic_chip = {
>> > .name = "GIC",
>> > .irq_mask = gic_mask_irq,
>> > .irq_unmask = gic_unmask_irq,
>> > .irq_eoi = gic_eoi_irq,
>> > .irq_set_type = gic_set_type,
>> > .irq_retrigger = gic_retrigger,
>> > #ifdef CONFIG_SMP
>> > .irq_set_affinity = gic_set_affinity,
>> > #endif
>> > .irq_set_wake = gic_set_wake,
>> > };
>> >
>> > Question is:
>> > When some drivers want to disable irq, maybe it will call
>> > linux standard inerface: irq_disable() that defined in include\
>> > linux\interrupt.h, this function will call
>> > void irq_disable(struct irq_desc *desc)
>> > {
>> > irq_state_set_disabled(desc);
>> > if (desc->irq_data.chip->irq_disable) {
>> > desc->irq_data.chip->irq_disable(&desc->irq_data);
>> > irq_state_set_masked(desc);
>> > }
>> > }
>> >
>> > Because gic have not register irq_diable method, so the interrupt can
>> > not disable immediately, it is enable until the interrupt come next time,
>> > then disalbed by mask because irq_state_set_disable(desc);
>> >
>> > I have checked irq_enable method:
>> > void irq_enable(struct irq_desc *desc)
>> > {
>> > irq_state_clr_disabled(desc);
>> > if (desc->irq_data.chip->irq_enable)
>> > desc->irq_data.chip->irq_enable(&desc->irq_data);
>> > else
>> > desc->irq_data.chip->irq_unmask(&desc->irq_data);
>> > irq_state_clr_masked(desc);
>> > }
>> > This method have do unmask when irq_enable method is not exist.
>> >
>> > Is it a good idea to add irq_mask call in irq_disable()?
>
> No. It's a very bad idea. We intentionally disable interrupts lazy.
>
> Thanks,
>
> tglx
Thanks,
I think this is lazy disable interrupts also. Could you give me
some guides about
how advantage about this?
But we can only lazy mask when that irq_data.chip have irq_diable()
and irq_mask(). If some
SOCs only have mask method, why not simply add irq_mask ? That is
simple and can reduce process time in future(Or If some rough drivers
havenot call diable wakeup then system deepsleep, because irq is
enable(if irq is enalbe, that have wakeup capacity), that could
unnecessary cause wakeup system).
If irq_disable only set flag, why irq_enable don't irq_unmask only
when irq flag is enable? That let the intc driver is unmatch invoked.
--
Steve Zhan
^ permalink raw reply
* [GIT PULL] clk: vt8500: Clock fixes for 3.8
From: Tony Prisk @ 2013-01-12 4:36 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mike,
The following changes since commit
a49f0d1ea3ec94fc7cf33a7c36a16343b74bd565:
Linux 3.8-rc1 (2012-12-21 17:19:00 -0800)
are available in the git repository at:
git://server.prisktech.co.nz/git/linuxwmt.git tags/clk-fixes-3.8
for you to fetch changes up to a3c2b58889440dcc6de92d04f09e8fcaf6bf7e2e:
clk: vt8500: Fix division-by-0 when requested rate=0 (2012-12-27
13:07:23 +1300)
----------------------------------------------------------------
clk: vt8500: Clock fixes for v3.8
Small series of fixes for clocks on vt8500.
Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
----------------------------------------------------------------
Tony Prisk (3):
clk: vt8500: Fix error in PLL calculations on non-exact match.
clk: vt8500: Fix device clock divisor calculations
clk: vt8500: Fix division-by-0 when requested rate=0
drivers/clk/clk-vt8500.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
^ permalink raw reply
* [PATCH 1/2] clk: Add debug clk summary
From: Mike Turquette @ 2013-01-12 3:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1356529583-19535-1-git-send-email-pgaikwad@nvidia.com>
Quoting Prashant Gaikwad (2012-12-26 05:46:22)
> Adds debug file "clk_summary" in /sys/kernel/debug/clk dir.
> It helps to view all the clock registered in human-readable format.
>
> For example:
> clock enable_cnt prepare_cnt rate
> ---------------------------------------------------------------------
> i2s0_sync 0 0 24000000
> spdif_in_sync 0 0 24000000
> spdif_mux 0 0 24000000
> spdif 0 0 24000000
> spdif_doubler 0 0 48000000
> spdif_div 0 0 48000000
> spdif_2x 0 0 48000000
> clk_32k 2 2 32768
> blink_override 1 1 32768
> blink 1 1 32768
> clk_m 2 2 12000000
> clk_out_3_mux 0 0 12000000
> clk_out_3 0 0 12000000
> pll_ref 3 3 12000000
> pll_e_mux 0 0 12000000
> pll_e 0 0 100000000
> cml0 0 0 100000000
> cml1 0 0 100000000
> pciex 0 0 100000000
> pll_d2 0 0 1000000
> pll_d2_out0 0 0 500000
> pll_d 0 0 1000000
> pll_d_out0 0 0 500000
> dsib_mux 0 0 500000
> dsib 0 0 500000
> dsia 0 0 500000
>
> Signed-off-by: Prashant Gaikwad <pgaikwad@nvidia.com>
Thanks for the respin of patch #1 and for generating the JSON output
format in patch #2. I've taken both of these into clk-next.
Regards,
Mike
> ---
> drivers/clk/clk.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 67 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 56e4495e..d79c088 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -34,6 +34,67 @@ static struct dentry *rootdir;
> static struct dentry *orphandir;
> static int inited = 0;
>
> +static void clk_summary_show_one(struct seq_file *s, struct clk *c, int level)
> +{
> + if (!c)
> + return;
> +
> + seq_printf(s, "%*s%-*s %-11d %-12d %-10lu",
> + level * 3 + 1, "",
> + 30 - level * 3, c->name,
> + c->enable_count, c->prepare_count, c->rate);
> + seq_printf(s, "\n");
> +}
> +
> +static void clk_summary_show_subtree(struct seq_file *s, struct clk *c,
> + int level)
> +{
> + struct clk *child;
> + struct hlist_node *tmp;
> +
> + if (!c)
> + return;
> +
> + clk_summary_show_one(s, c, level);
> +
> + hlist_for_each_entry(child, tmp, &c->children, child_node)
> + clk_summary_show_subtree(s, child, level + 1);
> +}
> +
> +static int clk_summary_show(struct seq_file *s, void *data)
> +{
> + struct clk *c;
> + struct hlist_node *tmp;
> +
> + seq_printf(s, " clock enable_cnt prepare_cnt rate\n");
> + seq_printf(s, "---------------------------------------------------------------------\n");
> +
> + mutex_lock(&prepare_lock);
> +
> + hlist_for_each_entry(c, tmp, &clk_root_list, child_node)
> + clk_summary_show_subtree(s, c, 0);
> +
> + hlist_for_each_entry(c, tmp, &clk_orphan_list, child_node)
> + clk_summary_show_subtree(s, c, 0);
> +
> + mutex_unlock(&prepare_lock);
> +
> + return 0;
> +}
> +
> +
> +static int clk_summary_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, clk_summary_show, inode->i_private);
> +}
> +
> +static const struct file_operations clk_summary_fops = {
> + .open = clk_summary_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> +};
> +
> /* caller must hold prepare_lock */
> static int clk_debug_create_one(struct clk *clk, struct dentry *pdentry)
> {
> @@ -167,12 +228,18 @@ static int __init clk_debug_init(void)
> {
> struct clk *clk;
> struct hlist_node *tmp;
> + struct dentry *d;
>
> rootdir = debugfs_create_dir("clk", NULL);
>
> if (!rootdir)
> return -ENOMEM;
>
> + d = debugfs_create_file("clk_summary", S_IRUGO, rootdir, NULL,
> + &clk_summary_fops);
> + if (!d)
> + return -ENOMEM;
> +
> orphandir = debugfs_create_dir("orphans", rootdir);
>
> if (!orphandir)
> --
> 1.7.4.1
^ permalink raw reply
* [PATCH] arm: exynos4: Fix suspend/resume hang
From: Thomas Abraham @ 2013-01-12 3:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357883278-26239-1-git-send-email-inderpal.singh@linaro.org>
On 10 January 2013 21:47, Inderpal Singh <inderpal.singh@linaro.org> wrote:
> commit d0341c61a2 removed the l2 cache save/restore from pm.c and commit
> 7c6035b63b added the l2 cache restoration in sleep.s to early enable the
> cache in resume path.
>
> With these changes the system hangs while resuming when vdd_arm
> is turned off in suspended state. The cache needs to be invalidated
> before turning it on, as it was being done before mentioned commits.
>
> Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org>
> ---
> arch/arm/mach-exynos/common.h | 1 +
> arch/arm/mach-exynos/pm.c | 4 ++++
> arch/arm/plat-samsung/s5p-sleep.S | 23 +++++++++++++++++++++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index af33872..056d9ce 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -78,6 +78,7 @@ enum sys_powerdown {
> };
>
> extern unsigned long l2x0_regs_phys;
> +extern unsigned long wake_reason_reg_phys;
> struct exynos_pmu_conf {
> void __iomem *reg;
> unsigned int val[NUM_SYS_POWERDOWN];
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index f459afd..b10513c 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -220,6 +220,10 @@ static __init int exynos_pm_drvinit(void)
>
> s3c_pm_init();
>
> +#ifdef CONFIG_CACHE_L2X0
> + if (!soc_is_exynos5250() && !soc_is_exynos5440())
> + wake_reason_reg_phys = virt_to_phys(S5P_INFORM1);
> +#endif
> /* All wakeup disable */
>
> tmp = __raw_readl(S5P_WAKEUP_MASK);
> diff --git a/arch/arm/plat-samsung/s5p-sleep.S b/arch/arm/plat-samsung/s5p-sleep.S
> index bdf6dad..74331e2 100644
> --- a/arch/arm/plat-samsung/s5p-sleep.S
> +++ b/arch/arm/plat-samsung/s5p-sleep.S
> @@ -24,6 +24,7 @@
> #include <linux/linkage.h>
> #include <asm/asm-offsets.h>
> #include <asm/hardware/cache-l2x0.h>
> +#include <mach/regs-pmu.h>
>
> /*
> * The following code is located into the .data section. This is to
> @@ -67,6 +68,25 @@ ENTRY(s3c_cpu_resume)
> str r2, [r1, #L2X0_PREFETCH_CTRL]
> ldr r2, [r0, #L2X0_R_PWR_CTRL]
> str r2, [r1, #L2X0_POWER_CTRL]
> +
> + /* check resume reason */
> + adr r0, wake_reason_reg_phys
> + ldr r0, [r0]
> + ldr r2, [r0]
> + ldr r0, =S5P_CHECK_SLEEP
> + cmp r0, r2
> + bne turn_cache_on
> +
> + /* Invalidate the cache if resume from sleep */
> + ldr r2, =0xffff
> + str r2, [r1, #L2X0_INV_WAY]
> +wait:
> + ldr r2, [r1, #L2X0_INV_WAY]
> + ldr r0, =0xffff
> + ands r2, r2, r0
> + bne wait
> +
> +turn_cache_on:
> mov r2, #1
> str r2, [r1, #L2X0_CTRL]
> resume_l2on:
> @@ -77,4 +97,7 @@ ENDPROC(s3c_cpu_resume)
> .globl l2x0_regs_phys
> l2x0_regs_phys:
> .long 0
> + .globl wake_reason_reg_phys
> +wake_reason_reg_phys:
> + .long 0
> #endif
Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org>
^ permalink raw reply
* [PATCH 4/4] ARM: uncompress: support uncompress debug for multiplatform build
From: Shawn Guo @ 2013-01-12 3:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111191116.GA999@quad.lixom.net>
On Fri, Jan 11, 2013 at 11:11:16AM -0800, Olof Johansson wrote:
> Hi,
>
> On Wed, Dec 12, 2012 at 09:45:45PM +0800, Shawn Guo wrote:
> > When moving to multiplatform build, platforms lose the output from
> > decompressor. Not only does it mean that we will never see message
> > "Uncompressing Linux... done, booting the kernel.", but also that
> > the debug facility in arch/arm/boot/compressed/head.S outputs nothing
> > even DEBUG is turned on.
> >
> > The patch follows the approach handling DEBUG_LL to create a Kconfig
> > symbol CONFIG_UNCOMPRESS_INCLUDE for choosing the correct uncompress
> > header. For traditional build, mach/uncompress.h will be included in
> > arch/arm/boot/compressed/misc.c. For multiplatform build with none
> > DEBUG_LL port enabled, debug/uncompress/dummy.h will be used. And
> > for multiplatform build with a DEBUG_LL port, platform specific
> > uncompress header like debug/uncompress/imx.h will come to play.
> >
> > Different from traditional build that uncompress output and DEBUG_LL
> > are two separated pieces, for multiplatform build uncompress output
> > becomes a subset of DEBUG_LL and will not work unless DEBUG_LL is
> > enabled.
> >
> > The arch/arm/include/debug/uncompress/imx.h is cleaned up and added
> > here as an example to show how this approach works.
>
> Since this requires you to define a debug uart to get uncompress output,
> it would make more sense to do a generic wrapper that uses the debug
> code generically, instead of having to add this kind of wrapper for
> every platform. That would solve it for all of them at once.
>
I'm not entirely sure about what you mean here. Can you please
elaborate that generic wrapper in your mind a little bit?
Shawn
^ permalink raw reply
* [PATCH 0/4] arm: vt8500: Add support for Wondermedia WM8750/WM8850
From: Tony Prisk @ 2013-01-12 3:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357960163.8515.3.camel@gitbox>
The following changes since commit
ce3f386fdf10d79eaf6ebd63bb7adbd95f08f9f0:
arm: vt8500: Remove remaining mach includes (2013-01-12 15:47:39
+1300)
are available in the git repository at:
git://server.prisktech.co.nz/git/linuxwmt.git tags/armsoc-3.9
for you to fetch changes up to 8d31bfa551b9a1a61119e6d8fde08d0fa8cdfafb:
arm: vt8500: Add support for Wondermedia WM8750/WM8850 (2013-01-12
15:51:24 +1300)
----------------------------------------------------------------
arm: vt8500: Add support for WM8750 and WM8850 SoCs.
This pull should be on top of:
git://server.prisktech.co.nz/git/linuxwmt.git
tags/vt8500-multiplatform-3.9
----------------------------------------------------------------
Tony Prisk (1):
arm: vt8500: Add support for Wondermedia WM8750/WM8850
Documentation/devicetree/bindings/arm/vt8500.txt | 8 ++++++++
arch/arm/mach-vt8500/Kconfig | 16
++++++++++++++++
arch/arm/mach-vt8500/vt8500.c | 2 ++
3 files changed, 26 insertions(+)
^ permalink raw reply
* [PATCH 0/4] arm: vt8500: Add support for Wondermedia WM8750/WM8850
From: Tony Prisk @ 2013-01-12 3:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1357960044.8515.0.camel@gitbox>
The following changes since commit
d1c3ed669a2d452cacfb48c2d171a1f364dae2ed:
Linux 3.8-rc2 (2013-01-02 18:13:21 -0800)
are available in the git repository at:
git://server.prisktech.co.nz/git/linuxwmt.git
tags/vt8500-multiplatform-3.9
for you to fetch changes up to ce3f386fdf10d79eaf6ebd63bb7adbd95f08f9f0:
arm: vt8500: Remove remaining mach includes (2013-01-12 15:47:39
+1300)
----------------------------------------------------------------
arm: vt8500: Convert arch-vt8500 to multiplatform only.
----------------------------------------------------------------
Tony Prisk (3):
arm: vt8500: Remove single platform Kconfig options
arm: vt8500: Convert debug-macro.S to be multiplatform friendly
arm: vt8500: Remove remaining mach includes
arch/arm/Kconfig | 16 ---------
arch/arm/Kconfig.debug | 8 +++++
.../mach/debug-macro.S => include/debug/vt8500.S} | 24 ++++++++-----
arch/arm/mach-vt8500/Kconfig | 11 ++++--
arch/arm/mach-vt8500/include/mach/timex.h | 26
--------------
arch/arm/mach-vt8500/include/mach/uncompress.h | 37
--------------------
6 files changed, 31 insertions(+), 91 deletions(-)
rename arch/arm/{mach-vt8500/include/mach/debug-macro.S =>
include/debug/vt8500.S} (56%)
delete mode 100644 arch/arm/mach-vt8500/include/mach/timex.h
delete mode 100644 arch/arm/mach-vt8500/include/mach/uncompress.h
^ permalink raw reply
* [PATCH 0/4] arm: vt8500: Add support for Wondermedia WM8750/WM8850
From: Tony Prisk @ 2013-01-12 3:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130111215659.GA16521@quad.lixom.net>
> I suggest that you split off the last three (and rebase them to be
> independent), and apply them to a multiplatform branch for vt8500. Then the
> last one is a soc branch that goes on top of the multiplatform branch (i.e. you
> build it on top of multiplatform). That way we can pull in the multiplatform
> changes together with those for other platforms, and then take the wm8{7,8}50
> branch goes in with the other new-soc support from other platforms.
>
> Does that make sense? I'll be happy to provide more explanation if it doesn't.
>
> -Olof
I think I understand - pull requests to follow.
Tony P
^ permalink raw reply
* [PATCH] clk: max77686: Remove unnecessary NULL checking for container_of()
From: Mike Turquette @ 2013-01-12 3:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1355817292.32595.2.camel@phoenix>
Quoting Axel Lin (2012-12-17 23:54:52)
> container_of() never returns NULL, thus remove the NULL checking for it.
> Also rename get_max77686_clk() to to_max77686_clk() for better readability.
>
> Signed-off-by: Axel Lin <axel.lin@gmail.com>
> ---
> drivers/clk/clk-max77686.c | 30 ++++++++----------------------
> 1 file changed, 8 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
> index 8944214..3680f66 100644
> --- a/drivers/clk/clk-max77686.c
> +++ b/drivers/clk/clk-max77686.c
> @@ -44,48 +44,34 @@ struct max77686_clk {
> struct clk_lookup *lookup;
> };
>
> -static struct max77686_clk *get_max77686_clk(struct clk_hw *hw)
> +static struct max77686_clk *to_max77686_clk(struct clk_hw *hw)
> {
> return container_of(hw, struct max77686_clk, hw);
> }
>
> static int max77686_clk_prepare(struct clk_hw *hw)
> {
> - struct max77686_clk *max77686;
> - int ret;
> -
> - max77686 = get_max77686_clk(hw);
> - if (!max77686)
> - return -ENOMEM;
> -
> - ret = regmap_update_bits(max77686->iodev->regmap,
> - MAX77686_REG_32KHZ, max77686->mask, max77686->mask);
> + struct max77686_clk *max77686 = to_max77686_clk(hw);
>
> - return ret;
> + return regmap_update_bits(max77686->iodev->regmap,
> + MAX77686_REG_32KHZ, max77686->mask,
> + max77686->mask);
> }
>
> static void max77686_clk_unprepare(struct clk_hw *hw)
> {
> - struct max77686_clk *max77686;
> -
> - max77686 = get_max77686_clk(hw);
> - if (!max77686)
> - return;
> + struct max77686_clk *max77686 = to_max77686_clk(hw);
>
> regmap_update_bits(max77686->iodev->regmap,
> - MAX77686_REG_32KHZ, max77686->mask, ~max77686->mask);
> + MAX77686_REG_32KHZ, max77686->mask, 0);
Looks like the change to ~max77686->mask snuck in. It is not mentioned
in the changelog. Care to respin the patch without the change?
Thanks,
Mike
> }
>
> static int max77686_clk_is_enabled(struct clk_hw *hw)
> {
> - struct max77686_clk *max77686;
> + struct max77686_clk *max77686 = to_max77686_clk(hw);
> int ret;
> u32 val;
>
> - max77686 = get_max77686_clk(hw);
> - if (!max77686)
> - return -ENOMEM;
> -
> ret = regmap_read(max77686->iodev->regmap,
> MAX77686_REG_32KHZ, &val);
>
> --
> 1.7.9.5
^ permalink raw reply
* [PATCH] clk: max77686: Fix return value checking for devm_kzalloc
From: Mike Turquette @ 2013-01-12 2:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1355816625.32595.0.camel@phoenix>
Quoting Axel Lin (2012-12-17 23:43:45)
> devm_kzalloc returns NULL on failure.
>
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
Pulled into clk-next.
Thanks for the patch,
Mike
> ---
> drivers/clk/clk-max77686.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
> index d098f72..8944214 100644
> --- a/drivers/clk/clk-max77686.c
> +++ b/drivers/clk/clk-max77686.c
> @@ -132,7 +132,7 @@ static int max77686_clk_register(struct device *dev,
>
> max77686->lookup = devm_kzalloc(dev, sizeof(struct clk_lookup),
> GFP_KERNEL);
> - if (IS_ERR(max77686->lookup))
> + if (!max77686->lookup)
> return -ENOMEM;
>
> max77686->lookup->con_id = hw->init->name;
> @@ -151,13 +151,13 @@ static int max77686_clk_probe(struct platform_device *pdev)
>
> max77686_clks = devm_kzalloc(&pdev->dev, sizeof(struct max77686_clk *)
> * MAX77686_CLKS_NUM, GFP_KERNEL);
> - if (IS_ERR(max77686_clks))
> + if (!max77686_clks)
> return -ENOMEM;
>
> for (i = 0; i < MAX77686_CLKS_NUM; i++) {
> max77686_clks[i] = devm_kzalloc(&pdev->dev,
> sizeof(struct max77686_clk), GFP_KERNEL);
> - if (IS_ERR(max77686_clks[i]))
> + if (!max77686_clks[i])
> return -ENOMEM;
> }
>
> --
> 1.7.9.5
^ permalink raw reply
* [PATCH] ARM: OMAP4: clock/hwmod data: remove MODULEMODE entries in mux + gate combos
From: Mike Turquette @ 2013-01-12 2:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1212172240101.29833@utopia.booyaka.com>
Quoting Paul Walmsley (2012-12-17 14:41:13)
> Convert all DEFINE_OMAP_MUX_GATE() combinations that list MODULEMODE
> registers in their gate arguments to DEFINE_OMAP_MUX(), dropping the
> MODULEMODE data. This is possible because the MODULEMODE bits control
> IP blocks, not clocks; and the hwmod code takes care of IP block
> control. Rename these clocks to reflect the original multiplexer name
> as specified in the comments. And convert the hwmod data to use the
> multiplexer clock name.
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Beno?t Cousson <b-cousson@ti.com>
> Cc: Mike Turquette <mturquette@linaro.org>
> ---
>
> Seems to boot with omap2plus_defconfig, anyway.
>
It looks good to me, and I am happy to see fewer clocks in favor of
using the hwmods. However I did not test it.
Regards,
Mike
^ permalink raw reply
* [PATCH] clk: factor: calculate rate by do_div
From: Mike Turquette @ 2013-01-12 2:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAN1soZzOy7K6u8Q-kCr0ZC9c2b8BBDpma_vbci1iFq7yCu1jqQ@mail.gmail.com>
Quoting Haojian Zhuang (2012-12-15 22:27:58)
> On Sun, Dec 16, 2012 at 4:54 AM, Mike Turquette <mturquette@linaro.org> wrote:
> > On Sat, Dec 15, 2012 at 8:41 AM, Haojian Zhuang
> > <haojian.zhuang@gmail.com> wrote:
> >> On Tue, Dec 4, 2012 at 9:32 AM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
> >>> On Mon, Dec 3, 2012 at 4:14 PM, Haojian Zhuang <haojian.zhuang@gmail.com> wrote:
> >>>> clk->rate = parent->rate / div * mult
> >>>>
> >>>> The formula is OK. But it may overflow while we do operate with
> >>>> unsigned long. So use do_div instead.
> >>>>
> >>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
> >>>> ---
> >>>> drivers/clk/clk-fixed-factor.c | 5 ++++-
> >>>> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
> >>>> index a489985..1ef271e 100644
> >>>> --- a/drivers/clk/clk-fixed-factor.c
> >>>> +++ b/drivers/clk/clk-fixed-factor.c
> >>>> @@ -28,8 +28,11 @@ static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
> >>>> unsigned long parent_rate)
> >>>> {
> >>>> struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
> >>>> + unsigned long long int rate;
> >>>>
> >>>> - return parent_rate * fix->mult / fix->div;
> >>>> + rate = (unsigned long long int)parent_rate * fix->mult;
> >>>> + do_div(rate, fix->div);
> >>>> + return (unsigned long)rate;
> >>>> }
> >>>>
> >>>> static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
> >>>> --
> >>>> 1.7.10.4
> >>>>
> >>>
> >>> Correct Mike's email address.
> >>
> >> Any comments? Does it mean that nobody want to fix the bug?
> >
> > Thanks for the patch. My apologies for letting this one slip through
> > the cracks but my normal email workflow was unavoidably disrupted and
> > I find myself playing catch-up with pending patches.
> >
> > The patch looks good to me but I'll change the $SUBJECT to "clk:
> > fixed-factor: round_rate should use do_div" and do some testing before
> > taking it in.
> >
> > Regards,
> > Mike
>
> It's nice. Thank you.
>
Pulled the fix into clk-next.
Thanks for the patch,
Mike
> Best Regards
> Haojian
^ permalink raw reply
* [PATCH] clk: export __clk_get_name for re-use in imx-ipu-v3 and others
From: Mike Turquette @ 2013-01-12 2:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121217085952.GH26326@pengutronix.de>
Quoting Sascha Hauer (2012-12-17 00:59:52)
> On Thu, Dec 13, 2012 at 01:12:25PM +0100, Niels de Vos wrote:
> > This fixes the following error when building for arm-imx:
> > > ERROR: "__clk_get_name" [drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.ko] undefined!
> > > make[1]: *** [__modpost] Error 1
> > > make: *** [modules] Error 2
> >
> > There are valid usecases to get the name of a clock, be it for debugging
> > purposes or to register a children of a clock like done in this IPU driver.
> > Therefore exporting __clk_get_name() and make it available for others makes
> > sense.
>
> Gnerally I think the name of a clock shouldn't be a secret, so:
>
> Acked-by: Sascha Hauer <s.hauer@pengutronix.de>
>
I've taken this into clk-next, but I'm not too pleased with it for any
purpose other than debugging. For this reason I opted to keep the
double underscores (as opposed to EXPORTing a new clk_get_name which
calls __clk_get_name) to make all users of the API feel uneasy and
otherwise of low character.
Currently the clock framework relies on parent names for registering
clocks from defined in static data but this limitation isn't really true
for clocks defined as part of DT. More to think about for the future.
Regards,
Mike
> >
> > Reported-by: Peter Robinson <pbrobinson@gmail.com>
> > CC: Sascha Hauer <s.hauer@pengutronix.de>
> > CC: Mike Turquette <mturquette@linaro.org>
> > Signed-off-by: Niels de Vos <ndevos@redhat.com>
> > ---
> > drivers/clk/clk.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index 251e45d..fbe0f3a 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -259,10 +259,11 @@ late_initcall(clk_disable_unused);
> >
> > /*** helper functions ***/
> >
> > -inline const char *__clk_get_name(struct clk *clk)
> > +const char *__clk_get_name(struct clk *clk)
> > {
> > return !clk ? NULL : clk->name;
> > }
> > +EXPORT_SYMBOL_GPL(__clk_get_name);
> >
> > inline struct clk_hw *__clk_get_hw(struct clk *clk)
> > {
> > --
> > 1.7.11.7
> >
> >
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox