* ftrace function_graph causes system crash
From: Jisheng Zhang @ 2016-09-21 9:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0e38f79fdc1f4671943da74764c15050@SIWEX5A.sing.micron.com>
Hi Bean,
On Wed, 21 Sep 2016 07:50:58 +0000 "Bean Huo (beanhuo)" wrote:
> > From: linux-arm-kernel [mailto:linux-arm-kernel-bounces at lists.infradead.org]
> > On Behalf Of Steven Rostedt
> > Sent: Dienstag, 20. September 2016 16:07
> > To: Bean Huo (beanhuo) <beanhuo@micron.com>
> > Cc: Zoltan Szubbocsev (zszubbocsev) <zszubbocsev@micron.com>;
> > catalin.marinas at arm.com; will.deacon at arm.com; rfi at lists.rocketboards.org;
> > linux-kernel at vger.kernel.org; mingo at redhat.com; linux-arm-
> > kernel at lists.infradead.org
> > Subject: Re: ftrace function_graph causes system crash
> >
> > On Tue, 20 Sep 2016 13:10:39 +0000
> > "Bean Huo (beanhuo)" <beanhuo@micron.com> wrote:
> >
> > > Hi, all
> > > I just use ftrace to do some latency study, found that function_graph
> > > can not Work, as long as enable it, will cause kernel panic. I searched this
> > online.
> > > Found that there are also some cause the same as mine. I am a newer of
> > ftrace.
> > > I want to know who know what root cause? Here is some partial log:
> > >
> > >
> >
> > Can you do a function bisect to find what function this is.
> >
> > This script is used to help find functions that are being traced by function tracer
> > or function graph tracing that causes the machine to reboot, hang, or crash.
> > Here's the steps to take.
> >
> > First, determine if function graph is working with a single function:
> >
> > # cd /sys/kernel/debug/tracing
> > # echo schedule > set_ftrace_filter
> > # echo function_graph > current_tracer
> >
> > If this works, then we know that something is being traced that shouldn't be.
> >
> > # echo nop > current_tracer
> >
> > # cat available_filter_functions > ~/full-file # ftrace-bisect ~/full-file ~/test-file
> > ~/non-test-file # cat ~/test-file > set_ftrace_filter
> >
> > *** Note *** this will take several minutes. Setting multiple functions is an
> > O(n^2) operation, and we are dealing with thousands of functions.
> > So go have coffee, talk with your coworkers, read facebook. And eventually,
> > this operation will end.
> >
> > # echo function_graph > current_tracer
> >
> > If it crashes, we know that ~/test-file has a bad function.
> >
> > Reboot back to test kernel.
> >
> > # cd /sys/kernel/debug/tracing
> > # mv ~/test-file ~/full-file
> >
> > If it didn't crash.
> >
> > # echo nop > current_tracer
> > # mv ~/non-test-file ~/full-file
> >
> > Get rid of the other test file from previous run (or save them off somewhere.
> > # rm -f ~/test-file ~/non-test-file
> >
> > And start again:
> >
> > # ftrace-bisect ~/full-file ~/test-file ~/non-test-file
> >
> > The good thing is, because this cuts the number of functions in ~/test-file by half,
> > the cat of it into set_ftrace_filter takes half as long each iteration, so don't talk
> > so much at the water cooler the second time.
> >
> > Eventually, if you did this correctly, you will get down to the problem function,
> > and all we need to do is to notrace it.
> >
> > The way to figure out if the problem function is bad, just do:
> >
> > # echo <problem-function> > set_ftrace_notrace # echo > set_ftrace_filter #
> > echo function_graph > current_tracer
> >
> > And if it doesn't crash, we are done.
> >
> > -- Steve
>
>
> Hi, Steve
> Thanks very much! This is a very useful trace tool, I now know the problem function,
> It is gt_counter_read, if not trace this function, ftrace function_graph work well.
I'm not sure whether the commit d6df3576e6b4 ("clocksource/drivers/arm_global_timer
: Prevent ftrace recursion") can fix this issue.
this commit is merged since v4.3, I noticed your kernel version is v4.0
Thanks,
Jisheng
> Do you know now how to deeply debug and trace which line is wrong through Ftrace?
>
> --Bean
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection
From: Jon Hunter @ 2016-09-21 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474443809.1239.4.camel@paulk.fr>
On 21/09/16 08:43, Paul Kocialkowski wrote:
...
>>>>>>> Depthcharge (the payload used with cros devices) will attempt to
>>>>>>> detect
>>>>>>> boards using their revision. This includes all the known revisions
>>>>>>> for
>>>>>>> the nyan-big board so that the dtb can be selected preferably.
>>>>>>
>>>>>> May be I am missing something here, but for the mainline there is only
>>>>>> one dtb available and so why is this needed for the mainline?
>>>>>
>>>>> There is indeed a single dts in mainline, but depthcharge will use the
>>>>> revision
>>>>> to match the compatible string (e.g. it will look for google,nyan-big-
>>>>> rev5,
>>>>> not
>>>>> google,nyan-big), so we need to list them all in that single dts.
>>>>> Otherwise,
>>>>> depthcharge will fall back to the default config, which may or may not
>>>>> be
>>>>> suitable for nyan.
>>>>
>>>> Is tegra124-nyan-big.dtb not the default?
>>>
>>> You can't expect that to always be the case. The image format allows many
>>> different dts to be provided, so I could easily build with
>>> multi_v7_defconfig
>>> and have various dts for various devices in the same image, and just select
>>> a
>>> random one as default.
>>
>> Really? Sounds odd. I was hoping that tegra124-nyan-big.dtb would be a
>> catch all.
I meant I was hoping that compatible = "google,nyan-big" would be the
catchall not the dtb file name ;-)
> Yes, the image format (FIT) allows specifying multiple dtb and zImage
> combinations in the same image[0].
Yes I am aware of that. Typically, I have been testing using a FIT image
with single zImage and dtb. Hence no problems.
So are you wanting to create a FIT image to support multiple boards and
use the single FIT image for all? If so then I can see why you want
this. Again please describe the motivation for the changes in the
changelog so it is clear why we are adding this.
Cheers
Jon
--
nvpublic
^ permalink raw reply
* [PATCH V3 1/4] ARM64 LPC: Indirect ISA port IO introduced
From: zhichang @ 2016-09-21 9:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5264074.nuyDhEuOR4@wuerfel>
Hi, Arnd,
On 2016?09?14? 22:23, Arnd Bergmann wrote:
> On Wednesday, September 14, 2016 10:16:28 PM CEST zhichang.yuan wrote:
>>>
>>> No need to guard includes with an #ifdef.
>> If remove #ifdef here, extio.h should not contain any function external declarations whose definitions are in
>> extio.c compiled only when CONFIG_ARM64_INDIRECT_PIO is yes.
>
> There is no problem with making declarations visible for functions that
> are not part of the kernel, we do that all the time.
>
>>>> +#define BUILDS_RW(bwl, type) \
>>>> +static inline void reads##bwl(const volatile void __iomem *addr, \
>>>> + void *buffer, unsigned int count) \
>>>> +{ \
>>>> + if (count) { \
>>>> + type *buf = buffer; \
>>>> + \
>>>> + do { \
>>>> + type x = __raw_read##bwl(addr); \
>>>> + *buf++ = x; \
>>>> + } while (--count); \
>>>> + } \
>>>> +} \
>>>> + \
>>>> +static inline void writes##bwl(volatile void __iomem *addr, \
>>>> + const void *buffer, unsigned int count) \
>>>> +{ \
>>>> + if (count) { \
>>>> + const type *buf = buffer; \
>>>> + \
>>>> + do { \
>>>> + __raw_write##bwl(*buf++, addr); \
>>>> + } while (--count); \
>>>> + } \
>>>> +}
>>>> +
>>>> +BUILDS_RW(b, u8)
>>>
>>> Why is this in here?
>> the readsb/writesb are defined in asm-generic/io.h which is included later, but the redefined insb/outsb need
>> to call them. Without these readsb/writesb definition before insb/outsb redefined, compile error occur.
>>
>> It seems that copy all the definitions of "asm-generic/io.h" is not a good idea, so I move the definitions of
>> those function needed here....
>>
>> Ok. I think your idea below defining in(s)/out(s) in a c file can solve this issue.
>>
>> #ifdef CONFIG_ARM64_INDIRECT_PIO
>> #define inb inb
>> extern u8 inb(unsigned long addr);
>>
>> #define outb outb
>> extern void outb(u8 value, unsigned long addr);
>>
>> #define insb insb
>> extern void insb(unsigned long addr, void *buffer, unsigned int count);
>>
>> #define outsb outsb
>> extern void outsb(unsigned long addr, const void *buffer, unsigned int count);
>> #endif
>>
>> and definitions of all these functions are in extio.c :
>>
>> u8 inb(unsigned long addr)
>> {
>> if (!arm64_extio_ops || arm64_extio_ops->start > addr ||
>> arm64_extio_ops->end < addr)
>> return readb(PCI_IOBASE + addr);
>> else
>> return arm64_extio_ops->pfin ?
>> arm64_extio_ops->pfin(arm64_extio_ops->devpara,
>> addr + arm64_extio_ops->ptoffset, NULL,
>> sizeof(u8), 1) : -1;
>> }
>> .....
>
> Yes, sounds good.
>
>>>> @@ -149,6 +185,60 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
>>>> #define IO_SPACE_LIMIT (PCI_IO_SIZE - 1)
>>>> #define PCI_IOBASE ((void __iomem *)PCI_IO_START)
>>>>
>>>> +
>>>> +/*
>>>> + * redefine the in(s)b/out(s)b for indirect-IO.
>>>> + */
>>>> +#define inb inb
>>>> +static inline u8 inb(unsigned long addr)
>>>> +{
>>>> +#ifdef CONFIG_ARM64_INDIRECT_PIO
>>>> + if (arm64_extio_ops && arm64_extio_ops->start <= addr &&
>>>> + addr <= arm64_extio_ops->end)
>>>> + return extio_inb(addr);
>>>> +#endif
>>>> + return readb(PCI_IOBASE + addr);
>>>> +}
>>>> +
>>>
>>> Looks ok, but you only seem to do this for the 8-bit
>>> accessors, when it should be done for 16-bit and 32-bit
>>> ones as well for consistency.
>> Hip06 LPC only support 8-bit I/O operations on the designated port.
>
> That is an interesting limitation. Maybe still call the extio operations
> and have them do WARN_ON_ONCE() instead?
>
> If you get a driver that calls inw/outw on the range that is owned
> by the LPC bus, you otherwise get an unhandled page fault in kernel
> space, which is not as nice.
As for this issue, I provided a wrong reply in the last email.
After double-checking with SoC guys, the inw(l)/outw(l) are OK with multiple 8-bit transfers to consecutive
I/O addresses.
Sorry for the wrong information!
Will support inw(l)/outw(l) in V4.
Best,
Zhichang
>
>>>> diff --git a/drivers/bus/extio.c b/drivers/bus/extio.c
>>>> new file mode 100644
>>>> index 0000000..1e7a9c5
>>>> --- /dev/null
>>>> +++ b/drivers/bus/extio.c
>>>> @@ -0,0 +1,66 @@
>>>
>>> This is in a globally visible directory
>>>
>>>> +
>>>> +struct extio_ops *arm64_extio_ops;
>>>
>>> But the identifier uses an architecture specific prefix. Either
>>> move the whole file into arch/arm64, or make the naming so that
>>> it can be used for everything.
>>
>> I perfer to move the whole file into arch/arm64, extio.h will be moved to arch/arm64/include/asm;
>
> Ok, that simplifies it a lot, you can just do everything in asm/io.h then.
>
> Arnd
>
^ permalink raw reply
* [PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions
From: Paul Kocialkowski @ 2016-09-21 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <eb7540c0-4abd-86c0-7777-78fe1f12bc59@nvidia.com>
Resending with the right CC chain.
Le mercredi 21 septembre 2016 ? 10:06 +0100, Jon Hunter a ?crit?:
> On 21/09/16 09:26, Paul Kocialkowski wrote:
> >
> >
> > * PGP Signed by an unknown key
> >
> > Le mercredi 21 septembre 2016 ? 08:52 +0100, Jon Hunter a ?crit :
> > >
> > >
> > > On 20/09/16 19:14, Paul Kocialkowski wrote:
> > > >
> > > >
> > > >
> > > > >
> > > > >
> > > > > Old Signed by an unknown key
> > > >
> > > > Le mardi 20 septembre 2016 ? 18:15 +0100, Jon Hunter a ?crit :
> > > > >
> > > > >
> > > > >
> > > > > On 28/08/16 18:32, Paul Kocialkowski wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > This switches a few interrupt definitions that were using
> > > > > > GPIO_ACTIVE_HIGH as IRQ type, which is invalid.
> > > > >
> > > > > May be you are right, but this does not describe why this is invalid.
> > > > > Can you elaborate?
> > > >
> > > > GPIO_ACTIVE_HIGH is simply not the right kind of define to use in the
> > > > "interrupts" devicetree property. Values provided there are understood
> > > > as
> > > > IRQ_TYPE_ defines.
> > >
> > > Right, but you are changing the type as GPIO_ACTIVE_HIGH = 0 and
> > > IRQ_TYPE_EDGE_FALLING = 2 and there is no comment about why this has
> > > been changed. It might be correct, but you need to explain it.
> >
> > This actually makes the IRQ trigger values consistent with the drivers, that
> > define them regardless of devicetree anyway. The max98090 driver
> > has IRQF_TRIGGER_FALLING and bq24735 has IRQF_TRIGGER_RISING |
> > IRQF_TRIGGER_FALLING.
> >
> > This is really more of a cosmetic change, it doesn't impact actual use.
>
> So you are saying that the drivers don't actually use the DT types?
It appears so. At least they are hardcoded in the kernel driver.
>
> ?May be that is ok, and yes this is cosmetic, but this should be stated in
> the changelog as it is not clear what is going on here.
Fair enough, will do in v2.
--
Paul Kocialkowski, developer of low-level free software for embedded devices
Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160921/c34cbaae/attachment-0001.sig>
^ permalink raw reply
* [PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection
From: Paul Kocialkowski @ 2016-09-21 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5461eb20-9372-7fa1-0e48-3d0143393b8c@nvidia.com>
Resending with the right CC chain.
Le mercredi 21 septembre 2016 ? 10:15 +0100, Jon Hunter a ?crit?:
>
> On 21/09/16 08:43, Paul Kocialkowski wrote:
>
> ...
>
> > > > > > > > Depthcharge (the payload used with cros devices) will attempt to
> > > > > > > > detect
> > > > > > > > boards using their revision. This includes all the known
> > > > > > > > revisions
> > > > > > > > for
> > > > > > > > the nyan-big board so that the dtb can be selected preferably.
> > > > > > >
> > > > > > > May be I am missing something here, but for the mainline there is
> > > > > > > only
> > > > > > > one dtb available and so why is this needed for the mainline?
> > > > > >
> > > > > > There is indeed a single dts in mainline, but depthcharge will use
> > > > > > the
> > > > > > revision
> > > > > > to match the compatible string (e.g. it will look for google,nyan-
> > > > > > big-
> > > > > > rev5,
> > > > > > not
> > > > > > google,nyan-big), so we need to list them all in that single dts.
> > > > > > Otherwise,
> > > > > > depthcharge will fall back to the default config, which may or may
> > > > > > not
> > > > > > be
> > > > > > suitable for nyan.
> > > > >
> > > > > Is tegra124-nyan-big.dtb not the default?
> > > >
> > > > You can't expect that to always be the case. The image format allows
> > > > many
> > > > different dts to be provided, so I could easily build with
> > > > multi_v7_defconfig
> > > > and have various dts for various devices in the same image, and just
> > > > select
> > > > a
> > > > random one as default.
> > >
> > > Really? Sounds odd. I was hoping that tegra124-nyan-big.dtb would be a
> > > catch all.
>
> I meant I was hoping that compatible = "google,nyan-big" would be the
> catchall not the dtb file name ;-)
Yeah, I figured :)
>
> >
> > Yes, the image format (FIT) allows specifying multiple dtb and zImage
> > combinations in the same image[0].
>
> Yes I am aware of that. Typically, I have been testing using a FIT image
> with single zImage and dtb. Hence no problems.
>
> So are you wanting to create a FIT image to support multiple boards and
> use the single FIT image for all? If so then I can see why you want
> this. Again please describe the motivation for the changes in the
> changelog so it is clear why we are adding this.
Fair enough, will do in v2.
--
Paul Kocialkowski, developer of low-level free software for embedded devices
Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160921/3e5c3e2d/attachment.sig>
^ permalink raw reply
* [PATCH v5 02/16] dt/bindings: Update binding for PM domain idle states
From: Brendan Jackman @ 2016-09-21 9:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160920161748.GB46914@linaro.org>
On Tue, Sep 20 2016 at 17:17, Lina Iyer <lina.iyer@linaro.org> wrote:
> On Mon, Sep 19 2016 at 09:09 -0600, Brendan Jackman wrote:
>>
>>On Fri, Sep 16 2016 at 18:39, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>> Hi Kevin,
>>>
>>> Thanks for looking at this and simplifying various discussions we had so
>>> far. I was thinking of summarizing something very similar. I couldn't
>>> due to lack of time.
>>>
>>> On 16/09/16 18:13, Kevin Hilman wrote:
>>>
>>> [...]
>>>
>>>> I think we're having some terminology issues...
>>>>
>>>> FWIW, the kernel terminolgy is actually "PM domain", not power domain.
>>>> This was intentional because the goal of the PM domain was to group
>>>> devices that some PM features. To be very specific to the kernel, they
>>>> us the same set of PM callbacks. Today, this is most commonly used to
>>>> model power domains, where a group of devices share a power rail, but it
>>>> does not need to be limited to that.
>>>>
>>>
>>> Agreed/Understood.
>>>
>>>> That being said, I'm having a hard time understanding the root of the
>>>> disagreement.
>>>>
>>>
>>> Yes. I tried to convey the same earlier, but have failed. The only
>>> disagreement is about a small part of this DT bindings. We would like to
>>> make it completely hierarchical up to CPU nodes. More comments on that
>>> below.
>>>
>>>> It seems that you and Sudeep would like to use domain-idle-states to
>>>> replace/superceed cpu-idle-states with the primary goal (and benefit)
>>>> being that it simplifies the DT bindings. Is that correct?
>>>>
>>>
>>> Correct, we want to deprecate cpu-idle-states with the introduction of
>>> this hierarchical PM bindings. Yes IMO, it simplifies things and avoids
>>> any ABI break we might trigger if we miss to consider some use-case now.
>>>
>>>> The objections have come in because that means that implies that CPUs
>>>> become their own domains, which may not be the case in hardware in the
>>>> sense that they share a power rail.
>>>>
>>>
>>> Agreed.
>>>
>>>> However, IMO, thinking of a CPU as it's own "PM domain" may make some
>>>> sense based on the terminology above.
>>>>
>>>
>>> Thanks for that, we do understand that it may not be 100% correct when
>>> we strictly considers hardware terminologies instead of above ones.
>>> As along as we see no issues with the above terminologies it should be fine.
>>>
>>>> I think the other objection may be that using a genpd to model domain
>>>> with only a single device in it may be overkill, and I agree with that.
>>>
>>> I too agree with that. Just because we represent that in DT in that way
>>> doesn't mean we need to create a genpd to model domain. We can always
>>> skip that if not required. That's pure implementation specifics and I
>>> have tried to convey the same in my previous emails. I must say you have
>>> summarized it very clearly in this email. Thanks again for that.
>>>
>>>> But, I'm not sure if making CPUs use domain-idle-states implies that
>>>> they necessarily have to use genpd is what you are proposing. Maybe
>>>> someone could clarify that?
>>>>
>>>
>>> No, I have not proposing anything around implementation in the whole
>>> discussion so far. I have constrained myself just to DT bindings so far.
>>> That's the main reason why I was opposed to mentions of OS vs platform
>>> co-ordinated modes of CPU suspend in this discussion. IMO that's
>>> completely out of scope of this DT binding we are defining here.
>>>
> Fair. But understand the PM Domain bindings do not impose any
> requirements of hierarchy. Domain idle states are defined by the
> property domain-idle-states in the domain node. How the DT bindings are
> organized is immaterial to the PM Domain core.
>
> It is a different exercise all together to look at CPU PSCI modes and
> have a unified way of representing them in DT. The current set of
> patches does not dictate where the domain idle states be located (pardon
> my example in the patch, which was not updated to reflect that). That
> said, I do require that domains that are controlled by the PSCI f/w be
> defined under the 'psci' node in DT, which is fair. All the domain needs
> are phandles to the idle state definitions; how the nodes are arranged
> in DT is not of consequence to the driver.
>
> In my mind providing a structure to CPU PM domains that can be used for
> both OSI and PC is a separate effort.
Do you mean a structure in the kernel or in DT? If the former, I agree,
if the latter I strongly disagree. I think DT bindings should be totally
unaware of PSCI suspend modes.
> It may also club what Brendan
> mentions below as part of the effort. The hierarchy that is presented in
> [1] is inherent in the PM domain hierarchy and idle states don't have to
> duplicate that information.
>
>>> Hope that helps/clarifies the misunderstanding/disagreement.
>>
>>Indeed. My intention was that the proposal would result in the exact
>>same kernel behaviour as Lina's current patchset, i.e. there is one
>>genpd per cluster, and CPU-level idle states are still handled by
>>cpuidle.
>>
>>The only change from the current patchset would be in initialisation
>>code: some coordination would need to be done to determine which idle
>>states go into cpuidle and which go into the genpds (whereas with the
>>current bindings, states from cpu-idle-states go into cpuidle and states
>>from domain-idle-states go into genpd). So you could say that this would
>>be a trade-off between binding simplicity and implementation simplicity.
>>
> I would not oppose the idea of virtual domains around CPUs (I admit I am
> not comfortable with the idea though), if that is the right thing to do.
> But the scope of that work is extensive and should not be clubbed as
> part of this proposal. It is an extensive code rework spanning cpuidle
> drivers and PSCI and there are hooks in this code to help you achieve
> that.
If we want to take the per-CPU domains approach, we _have_ to do it as
part of this proposal; it's a different set of semantics for the
cpu-idle-states/domain-idle-states properties. It would mean
cpu-idle-states is _superseded_ by domain idle states - implementing one
solution (where cpu-idle-states and domain idle states are both taken
into consideration by the implementation) then later switching to the
alternative (where cpu-idle-states is ignored when a CPU PM domain tree
is present) wouldn't make sense from a backward-compatibility
perspective.
You're right that implementing the alternative proposal in the Linux
kernel would mean quite a big rework. But, idealistically speaking,
Linux-specific implementation realities shouldn't be a factor in Device
Tree binding designs, right?
If people think that using both cpu-idle-states and domain-idle-states
is the pragmatic choice (or object fundamentally to the idea of devices
with idle states as being in their own PM domain) then that's fine IMO,
but it's a one-time decision and I think we should be clear about why
we're making it.
Cheers,
Brendan
^ permalink raw reply
* [PATCH] gpio: Added zynq specific check for special pins on bank zero
From: Shubhrajyoti Datta @ 2016-09-21 9:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474360324-8168-1-git-send-email-navam@xilinx.com>
On Tue, Sep 20, 2016 at 2:02 PM, Nava kishore Manne
<nava.manne@xilinx.com> wrote:
> From: Nava kishore Manne <nava.manne@xilinx.com>
>
> This patch adds zynq specific check for bank 0 pins 7 and 8
> are special and cannot be used as inputs
Is there any such pins for zynqmp?
>
> Signed-off-by: Nava kishore Manne <navam@xilinx.com>
> ---
> drivers/gpio/gpio-zynq.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c
> index e72794e..eae9d24 100644
> --- a/drivers/gpio/gpio-zynq.c
> +++ b/drivers/gpio/gpio-zynq.c
> @@ -96,6 +96,10 @@
> /* GPIO upper 16 bit mask */
> #define ZYNQ_GPIO_UPPER_MASK 0xFFFF0000
>
> +/* For GPIO quirks */
> +#define ZYNQ_GPIO BIT(0)
> +#define ZYNQMP_GPIO BIT(1)
if not can we remove ZYNQMP_GPIO ?
^ permalink raw reply
* [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06
From: zhichang @ 2016-09-21 10:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <26171742.fWcrANu6EP@wuerfel>
Hi, Arnd,
On 2016?09?15? 20:24, Arnd Bergmann wrote:
> On Thursday, September 15, 2016 12:05:51 PM CEST Gabriele Paoloni wrote:
>>> -----Original Message-----
>>> On Thursday, September 15, 2016 8:02:27 AM CEST Gabriele Paoloni wrote:
>>>>
>>>> From <<3.1.1. Open Firmware Properties for Bus Nodes>> in
>>>> http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
>>>>
>>>> I quote:
>>>> "There shall be an entry in the "ranges" property for each
>>>> of the Memory and/or I/O spaces if that address space is
>>>> mapped through the bridge."
>>>>
>>>> It seems to me that it is ok to have 1:1 address mapping and that
>>>> therefore of_translate_address() should fail if "ranges" is not
>>>> present.
>>>
>>> The key here is the definition of "mapped through the bridge".
>>> I can only understand this as "directly mapped", i.e. an I/O
>>> port of the child bus corresponds directly to a memory address
>>> on the parent bus, but this is not the case here.
>>>
>>> The problem with adding the mapping here is that it looks
>>> like it should be valid to create a page table entry for
>>> the address returned from the translation and access it through
>>> a pointer dereference, but that is clearly not possible.
>>
>> I understand that somehow we are abusing of the ranges property
>> here however the point is that with the current implementation ranges
>> is needed because otherwise the ipmi driver probe will fail here:
>>
>> of_ipmi_probe -> of_address_to_resource -> __of_address_to_resource
>> -> of_translate_address -> __of_translate_address
>>
>> Now we had a bit of discussion internally and to avoid
>> having ranges we came up with two possible solutions:
>>
>> 1) Using bit 3 of phys.hi cell in 2.2.1 of
>> http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
>> This would mean reworking of_bus_isa_get_flags in
>> http://lxr.free-electrons.com/source/drivers/of/address.c#L398
>> and setting a new flag to be checked in __of_address_to_resource
>>
>> 2) Adding a property in the bindings of each device that is
>> a child of our LPC bus and modify __of_address_to_resource
>> to check if the property is in the DT and eventually bypass
>> of_translate_address
>>
>> However in both 1) and 2) there are some issues:
>> in 1) we are not complying with the isa binding doc (we use
>> a bit that should be zero); in 2) we need to modify the
>> bindings documentation of the devices that are connected
>> to our LPC controller (therefore modifying other devices
>> bindings to fit our special case).
>>
>> I think that maybe having the 1:1 range mapping doesn't
>> reflect well the reality but it is the less painful
>> solution...
>>
>> What's your view?
>
> We can check the 'i' bit for I/O space in of_bus_isa_get_flags,
> and that should be enough to translate the I/O port number.
>
> The only part we need to change here is to not go through
> the crazy conversion all the way from PCI I/O space to a
> physical address and back to a (logical) port number
> that we do today with of_translate_address/pci_address_to_pio.
>
Sorry for the late response! Several days' leave....
Do you want to bypass of_translate_address and pci_address_to_pio for the registered specific PIO?
I think the bypass for of_translate_address is ok, but worry some new issues will emerge without the
conversion between physical address and logical/linux port number.
When PCI host bridge which support IO operations is configured and enabled, the pci_address_to_pio will
populate the logical IO range from ZERO for the first host bridge. Our LPC will also use part of the IO range
started from ZERO. It will make in/out enter the wrong branch possibly.
In V2, the 0 - 0x1000 logical IO range is reserved for LPC use only. But it seems not so good. In this way,
PCI has no chance to use low 4K IO range(logical).
So, in V3, applying the conversion from physical/cpu address to logical/linux IO port for any IO ranges,
including the LPC, but recorded the logical IO range for LPC. When calling in/out with a logical port address,
we can check this port fall into LPC logical IO range and get back the real IO.
Do you have further comments about this??
> I can think of a several of ways to fix __of_address_to_resource
> to just do the right thing according to the ISA binding to
> make the normal drivers work.
>
> The easiest solution is probably to hook into the
> "taddr == OF_BAD_ADDR" case in __of_address_to_resource
> and add a lookup for ISA buses there, and instead check
> if some special I/O port operations were registered
> for the port number, using an architecture specific
> function that arm64 implements. Other architectures
> like x86 that don't have a direct mapping between I/O
> ports and MMIO addresses would implement that same
> function differently.
What about add the specific quirk for Hip06 LPC in of_empty_ranges_quirk()??
you know, there are several cases in which of_translate_address return OF_BAD_ADDR.
And if we only check the special port range, it seems a bit risky. If some device want to use this port range
when no hip06 LPC is configured, the checking does not work. I think we should also check the relevant device.
Best,
Zhichang
>
> Arnd
>
^ permalink raw reply
* [PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger
From: Jon Hunter @ 2016-09-21 10:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474444619.1239.13.camel@paulk.fr>
On 21/09/16 08:56, Paul Kocialkowski wrote:
...
> Sure, this is exported at: /sys/class/power_supply/bq24735 at 5-0009
> Also, note that the power-supply next branch[2] has some more fixes for the
> bq24735 driver.
I tested the bq24735 on next-20160919 without any of your changes and
when connecting or disconnecting the charger I see the following panic.
Do you see this?
Jon
/ # [ 30.120384] Unable to handle kernel NULL pointer dereference at virtual address 00000004
[ 30.128489] pgd = c0004000
[ 30.131187] [00000004] *pgd=00000000
[ 30.134759] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
[ 30.140141] Modules linked in:
[ 30.143192] CPU: 1 PID: 71 Comm: kworker/1:1 Not tainted 4.8.0-rc6-next-20160919-00002-gbc9771827865-dirty #574
[ 30.153254] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[ 30.159509] Workqueue: events power_supply_changed_work
[ 30.164723] task: ee19f880 task.stack: ee1a0000
[ 30.169239] PC is at sbs_external_power_changed+0x50/0x5c
[ 30.174624] LR is at mod_timer+0x194/0x270
[ 30.178705] pc : [<c05b0c58>] lr : [<c017cda0>] psr: 60000113
[ 30.178705] sp : ee1a1eb8 ip : 80000000 fp : ee8e3680
[ 30.190155] r10: eef98580 r9 : 00000000 r8 : 00000000
[ 30.195363] r7 : ee303000 r6 : c05afef8 r5 : ee3e03f8 r4 : ee3e03d0
[ 30.201872] r3 : 00000000 r2 : 00000000 r1 : 40000193 r0 : 00000001
[ 30.208382] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 30.215497] Control: 10c5387d Table: adcf006a DAC: 00000051
[ 30.221226] Process kworker/1:1 (pid: 71, stack limit = 0xee1a0210)
[ 30.227474] Stack: (0xee1a1eb8 to 0xee1a2000)
[ 30.231816] 1ea0: edc0bc00 ee303000
[ 30.239973] 1ec0: c05afef8 c05aff2c edc0bc20 c045ec1c eef95c80 eef95c80 00000001 eea82d5c
[ 30.248136] 1ee0: edc0bd98 00000000 ee3031c0 ee303218 c0eab740 c05afcc4 ee8e3680 ee3031c0
[ 30.256296] 1f00: eef9b800 00000000 eef98580 c0135824 eef98598 c0e02100 eef98580 ee8e3698
[ 30.264453] 1f20: 00000008 eef98598 c0e02100 ee1a0000 eef98580 c0135a74 ee840f80 ee8e3680
[ 30.272610] 1f40: c0135a3c 00000000 ee840f80 ee8e3680 c0135a3c 00000000 00000000 00000000
[ 30.280766] 1f60: 00000000 c013af18 00000000 00000000 00000000 ee8e3680 00000000 00000000
[ 30.288922] 1f80: ee1a1f80 ee1a1f80 00000000 00000000 ee1a1f90 ee1a1f90 ee1a1fac ee840f80
[ 30.297078] 1fa0: c013ae3c 00000000 00000000 c01078b8 00000000 00000000 00000000 00000000
[ 30.305234] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 30.313389] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 30.321555] [<c05b0c58>] (sbs_external_power_changed) from [<c05aff2c>] (__power_supply_changed_work+0x34/0x3c)
[ 30.331623] [<c05aff2c>] (__power_supply_changed_work) from [<c045ec1c>] (class_for_each_device+0x4c/0xb4)
[ 30.341254] [<c045ec1c>] (class_for_each_device) from [<c05afcc4>] (power_supply_changed_work+0x5c/0xb0)
[ 30.350713] [<c05afcc4>] (power_supply_changed_work) from [<c0135824>] (process_one_work+0x124/0x33c)
[ 30.359912] [<c0135824>] (process_one_work) from [<c0135a74>] (worker_thread+0x38/0x4d4)
[ 30.367983] [<c0135a74>] (worker_thread) from [<c013af18>] (kthread+0xdc/0xf4)
[ 30.375187] [<c013af18>] (kthread) from [<c01078b8>] (ret_from_fork+0x14/0x3c)
[ 30.382391] Code: e5911000 e3a00004 ebee0b88 e5943008 (e5933004)
[ 30.388504] ---[ end trace 083d55597e9a2254 ]---
[ 30.393140] Unable to handle kernel paging request at virtual address ffffffec
[ 30.400342] pgd = c0004000
[ 30.403038] [ffffffec] *pgd=afffd861, *pte=00000000, *ppte=00000000
[ 30.409309] Internal error: Oops: 37 [#2] PREEMPT SMP ARM
[ 30.414690] Modules linked in:
[ 30.417738] CPU: 1 PID: 71 Comm: kworker/1:1 Tainted: G D 4.8.0-rc6-next-20160919-00002-gbc9771827865-dirty #574
[ 30.429013] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[ 30.435265] task: ee19f880 task.stack: ee1a0000
[ 30.439782] PC is at kthread_data+0x4/0xc
[ 30.443779] LR is at wq_worker_sleeping+0x8/0xc8
[ 30.448381] pc : [<c013b8a8>] lr : [<c01366e0>] psr: 20000193
[ 30.448381] sp : ee1a1c58 ip : eef98f28 fp : ee1a1cb4
[ 30.459836] r10: eef98a00 r9 : c0e5f000 r8 : c0d9ea00
[ 30.465048] r7 : ee19fcc8 r6 : c0e02e08 r5 : ee19f880 r4 : eef98a00
[ 30.471556] r3 : 00000000 r2 : 00000020 r1 : 00000000 r0 : ee19f880
[ 30.478066] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
[ 30.485268] Control: 10c5387d Table: adcf006a DAC: 00000051
[ 30.490998] Process kworker/1:1 (pid: 71, stack limit = 0xee1a0210)
[ 30.497247] Stack: (0xee1a1c58 to 0xee1a2000)
[ 30.501590] 1c40: eef98a00 c0833a68
[ 30.509747] 1c60: 00000001 ee117a44 c0d99280 a0000193 00000000 c0833d4c 00000001 ee117a44
[ 30.517904] 1c80: 00000000 ee117540 ee19f880 c01faf3c 00000000 ee1a0000 ee1a18ec 00000001
[ 30.526060] 1ca0: ee1a1cc8 ee19fc40 c05b0c5c 00000001 ee1a1cc4 c0833d4c ee19f880 ee1a18ec
[ 30.534217] 1cc0: ee85d3c0 c012277c ee1a1cc8 ee1a1cc8 00000000 c0e5e2c4 c0e07638 ee1a1e68
[ 30.542374] 1ce0: 60000113 0000000b c05b0c5c 00000001 c05b0c5a c010b6f8 ee1a0210 0000000b
[ 30.550530] 1d00: c0e07638 ee1a0000 bf000000 00000008 65000000 31313935 20303030 30613365
[ 30.558687] 1d20: 34303030 65626520 38623065 35652038 30333439 28203830 33393565 34303033
[ 30.566843] 1d40: 00002029 c01b63c4 c0004000 00000004 ee1a1e68 00000017 00000000 00000017
[ 30.575000] 1d60: 00000004 eef98580 ee8e3680 c011a424 00000004 c0115c20 ee19f880 00000000
[ 30.583156] 1d80: ee19f900 ee2ecd80 ee19f880 c014a55c c014a460 ee19f880 eef98a00 00000000
[ 30.591312] 1da0: a0000193 c0e07efc 00000017 c0115888 00000004 ee1a1e68 ee1a0000 eef98580
[ 30.599469] 1dc0: ee8e3680 c01012ac 1514622a 00000000 ee19f900 c0149578 eef98a00 ee19f900
[ 30.607626] 1de0: 00000001 00000000 eef98a38 00000000 000043e1 c014e11c ee2ecd00 ee19f900
[ 30.615781] 1e00: 00000010 00000000 00000006 00000000 00000000 edc45440 c0d992c4 00000000
[ 30.623938] 1e20: 00000001 00000000 00000000 00000001 00004b4e 00000000 00000001 c0e5f000
[ 30.632095] 1e40: 00000000 00000000 ee19f900 ee19f900 c05b0c58 60000113 ffffffff ee1a1e9c
[ 30.640251] 1e60: 00000000 c010bd78 00000001 40000193 00000000 00000000 ee3e03d0 ee3e03f8
[ 30.648408] 1e80: c05afef8 ee303000 00000000 00000000 eef98580 ee8e3680 80000000 ee1a1eb8
[ 30.656566] 1ea0: c017cda0 c05b0c58 60000113 ffffffff 00000051 00000000 edc0bc00 ee303000
[ 30.664722] 1ec0: c05afef8 c05aff2c edc0bc20 c045ec1c eef95c80 eef95c80 00000001 eea82d5c
[ 30.672886] 1ee0: edc0bd98 00000000 ee3031c0 ee303218 c0eab740 c05afcc4 ee8e3680 ee3031c0
[ 30.681045] 1f00: eef9b800 00000000 eef98580 c0135824 eef98598 c0e02100 eef98580 ee8e3698
[ 30.689202] 1f20: 00000008 eef98598 c0e02100 ee1a0000 eef98580 c0135a74 ee840f80 ee8e3680
[ 30.697358] 1f40: c0135a3c 00000000 ee840f80 ee8e3680 c0135a3c 00000000 00000000 00000000
[ 30.705515] 1f60: 00000000 c013af18 00000000 00000000 00000000 ee8e3680 00000000 00000000
[ 30.713672] 1f80: ee1a1f80 ee1a1f80 00000001 00010001 ee1a1f90 ee1a1f90 ee1a1fac ee840f80
[ 30.721827] 1fa0: c013ae3c 00000000 00000000 c01078b8 00000000 00000000 00000000 00000000
[ 30.729983] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 30.738140] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 30.746304] [<c013b8a8>] (kthread_data) from [<c01366e0>] (wq_worker_sleeping+0x8/0xc8)
[ 30.754292] [<c01366e0>] (wq_worker_sleeping) from [<c0833a68>] (__schedule+0x45c/0x6f0)
[ 30.762364] [<c0833a68>] (__schedule) from [<c0833d4c>] (schedule+0x50/0xb4)
[ 30.769394] [<c0833d4c>] (schedule) from [<c012277c>] (do_exit+0x6e8/0xa90)
[ 30.776340] [<c012277c>] (do_exit) from [<c010b6f8>] (die+0x470/0x488)
[ 30.782853] [<c010b6f8>] (die) from [<c011a424>] (__do_kernel_fault.part.0+0x64/0x1e4)
[ 30.790751] [<c011a424>] (__do_kernel_fault.part.0) from [<c0115c20>] (do_page_fault+0x398/0x3a4)
[ 30.799601] [<c0115c20>] (do_page_fault) from [<c01012ac>] (do_DataAbort+0x38/0xb8)
[ 30.807238] [<c01012ac>] (do_DataAbort) from [<c010bd78>] (__dabt_svc+0x58/0x80)
[ 30.814613] Exception stack(0xee1a1e68 to 0xee1a1eb0)
[ 30.819650] 1e60: 00000001 40000193 00000000 00000000 ee3e03d0 ee3e03f8
[ 30.827807] 1e80: c05afef8 ee303000 00000000 00000000 eef98580 ee8e3680 80000000 ee1a1eb8
[ 30.835962] 1ea0: c017cda0 c05b0c58 60000113 ffffffff
[ 30.841002] [<c010bd78>] (__dabt_svc) from [<c05b0c58>] (sbs_external_power_changed+0x50/0x5c)
[ 30.849593] [<c05b0c58>] (sbs_external_power_changed) from [<c05aff2c>] (__power_supply_changed_work+0x34/0x3c)
[ 30.859660] [<c05aff2c>] (__power_supply_changed_work) from [<c045ec1c>] (class_for_each_device+0x4c/0xb4)
[ 30.869291] [<c045ec1c>] (class_for_each_device) from [<c05afcc4>] (power_supply_changed_work+0x5c/0xb0)
[ 30.878762] [<c05afcc4>] (power_supply_changed_work) from [<c0135824>] (process_one_work+0x124/0x33c)
[ 30.887963] [<c0135824>] (process_one_work) from [<c0135a74>] (worker_thread+0x38/0x4d4)
[ 30.896034] [<c0135a74>] (worker_thread) from [<c013af18>] (kthread+0xdc/0xf4)
[ 30.903240] [<c013af18>] (kthread) from [<c01078b8>] (ret_from_fork+0x14/0x3c)
[ 30.910443] Code: e34c00a4 ebff90d2 eafffff2 e5903418 (e5130014)
[ 30.916520] ---[ end trace 083d55597e9a2255 ]---
[ 30.921122] Fixing recursive fault but reboot is needed!
--
nvpublic
^ permalink raw reply
* [RFC] irqchip/gic-v3: Implement suspend and resume callbacks
From: Marc Zyngier @ 2016-09-21 10:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474447359-29551-1-git-send-email-clingutla@codeaurora.org>
+Sudeep, Lorenzo,
On 21/09/16 09:42, Lingutla Chandrasekhar wrote:
> Implement suspend and resume syscore_ops to disable and
> enable non wake up capable interrupts.
>
> When system enters suspend, enable only wakeup capable
> interrupts. While resuming, enable previously enabled interrupts
> and show triggered/pending interrupts.
The fundamental problem (which you're not mentioning at all) is that the
GICv3 architecture doesn't mention wake-up interrupts at all, so this
has to be entirely handled in firmware. Is that what is happening?
>
> Signed-off-by: Lingutla Chandrasekhar <clingutla@codeaurora.org>
>
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index ede5672..511a5a1 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -33,6 +33,7 @@
> #include <linux/irqchip/arm-gic-common.h>
> #include <linux/irqchip/arm-gic-v3.h>
> #include <linux/irqchip/irq-partition-percpu.h>
> +#include <linux/syscore_ops.h>
>
> #include <asm/cputype.h>
> #include <asm/exception.h>
> @@ -57,6 +58,10 @@ struct gic_chip_data {
> u32 nr_redist_regions;
> unsigned int irq_nr;
> struct partition_desc *ppi_descs[16];
> +#ifdef CONFIG_PM
> + unsigned int wakeup_irqs[32];
> + unsigned int enabled_irqs[32];
Do not use ambiguous types for something that comes from the HW. Where
does this '32' comes from?
> +#endif
> };
>
> static struct gic_chip_data gic_data __read_mostly;
> @@ -330,6 +335,81 @@ static int gic_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu)
> return 0;
> }
>
> +#ifdef CONFIG_PM
> +static int gic_suspend(void)
> +{
> + unsigned int i;
> + void __iomem *base = gic_data.dist_base;
> +
> + for (i = 0; i * 32 < gic->irq_nr; i++) {
> + gic->enabled_irqs[i]
> + = readl_relaxed(base + GICD_ISENABLER + i * 4);
Do you realize that GICD_ISENABLER0 is always zero? What do you do for
PPIs? Please keep the assignment on a single line.
> + /* disable all of them */
> + writel_relaxed(0xffffffff, base + GICD_ICENABLER + i * 4);
> + /* enable the wakeup set */
> + writel_relaxed(gic->wakeup_irqs[i],
> + base + GICD_ISENABLER + i * 4);
On a single line as well.
> + }
> + return 0;
> +}
> +
> +static void gic_show_pending(void)
> +{
> + unsigned int i;
> + u32 enabled;
> + u32 pending[32];
> + void __iomem *base = gic_data.dist_base;
> +
> + for (i = 0; i * 32 < gic->irq_nr; i++) {
> + enabled = readl_relaxed(base + GICD_ICENABLER + i * 4);
> + pending[i] = readl_relaxed(base + GICD_ISPENDR + i * 4);
> + pending[i] &= enabled;
> + }
> +
> + for_each_set_bit(i, (unsigned long *)pending, gic->irq_nr) {
> + unsigned int irq = irq_find_mapping(gic->domain, i);
> + struct irq_desc *desc = irq_to_desc(irq);
> + const char *name = "null";
> +
> + if (desc == NULL)
> + name = "stray irq";
> + else if (desc->action && desc->action->name)
> + name = desc->action->name;
> +
> + pr_debug("Pending IRQ: %d [%s]\n", __func__, irq, name);
> + }
> +}
Please drop this function from this patch, it doesn't serve any purpose
other than your own debugging.
> +
> +static void gic_resume(void)
> +{
> + unsigned int i;
> + void __iomem *base = gic_data.dist_base;
> +
> + gic_show_pending();
> +
> + for (i = 0; i * 32 < gic->irq_nr; i++) {
> + /* disable all of them */
> + writel_relaxed(0xffffffff, base + GICD_ICENABLER + i * 4);
> + /* enable the enabled set */
> + writel_relaxed(gic->enabled_irqs[i],
> + base + GICD_ISENABLER + i * 4);
Same remarks as the suspend side.
> + }
> +}
> +
> +static struct syscore_ops gic_syscore_ops = {
> + .suspend = gic_suspend,
> + .resume = gic_resume,
> +};
> +
> +static int __init gic_init_sys(void)
> +{
> + register_syscore_ops(&gic_syscore_ops);
> + return 0;
> +}
> +device_initcall(gic_init_sys);
> +
> +#endif
> +
> static u64 gic_mpidr_to_affinity(unsigned long mpidr)
> {
> u64 aff;
> @@ -666,6 +746,32 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,
> #define gic_smp_init() do { } while(0)
> #endif
>
> +#ifdef CONFIG_PM
> +int gic_set_wake(struct irq_data *d, unsigned int on)
> +{
> + int ret = -ENXIO;
> + unsigned int reg_offset, bit_offset;
> + unsigned int gicirq = gic_irq(d);
> + struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> +
> + /* per-cpu interrupts cannot be wakeup interrupts */
> + WARN_ON(gicirq < 32);
How did you decide that? There is no such specification anywhere.
> +
> + reg_offset = gicirq / 32;
> + bit_offset = gicirq % 32;
> +
> + if (on)
> + gic_data->wakeup_irqs[reg_offset] |= 1 << bit_offset;
> + else
> + gic_data->wakeup_irqs[reg_offset] &= ~(1 << bit_offset);
> +
> + return ret;
> +}
> +
> +#else
> +#define gic_set_wake NULL
> +#endif
> +
> #ifdef CONFIG_CPU_PM
> /* Check whether it's single security state view */
> static bool gic_dist_security_disabled(void)
> @@ -707,6 +813,7 @@ static struct irq_chip gic_chip = {
> .irq_eoi = gic_eoi_irq,
> .irq_set_type = gic_set_type,
> .irq_set_affinity = gic_set_affinity,
> + .irq_set_wake = gic_set_wake,
> .irq_get_irqchip_state = gic_irq_get_irqchip_state,
> .irq_set_irqchip_state = gic_irq_set_irqchip_state,
> .flags = IRQCHIP_SET_TYPE_MASKED,
> @@ -723,6 +830,7 @@ static struct irq_chip gic_eoimode1_chip = {
> .irq_set_irqchip_state = gic_irq_set_irqchip_state,
> .irq_set_vcpu_affinity = gic_irq_set_vcpu_affinity,
> .flags = IRQCHIP_SET_TYPE_MASKED,
> + .irq_set_wake = gic_set_wake,
Keep the fields in the same order.
> };
>
> #define GIC_ID_NR (1U << gic_data.rdists.id_bits)
>
But here's my fundamental objection: None of that should be required and
setting (IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND) as part of the
irqchip flags should be enough.
Can you explain why this doesn't work for you?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH] pinctrl: mvebu: orion5x: Generalise mv88f5181l support for 88f5181
From: Gregory CLEMENT @ 2016-09-21 10:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdbo6JmR=K58KQ_J0Gd5_gLqub65watU-iEabQ0rWdeVpQ@mail.gmail.com>
Hi Linus,
On mar., sept. 20 2016, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Sep 20, 2016 at 6:26 PM, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
>
>> From: Jamie Lentin <jm@lentin.co.uk>
>>
>> As far as I'm aware the mv88f5181-b1 and mv88f5181l are the same at the
>> pinctrl level, so re-use the definitions for both.
>>
>> [gregory.clement at free-electrons.com: fix commit title]
>> Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> Is this the patch from your pull request?
yes this one.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>
> Or do you prefer that I take it into the pinctrl tree?
No it is ok, it will be in the second tentaive of my pull request.
Thanks,
Gregory
>
> Yours,
> Linus Walleij
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH V3 3/4] ARM64 LPC: support serial based on low-pin-count
From: zhichang @ 2016-09-21 10:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5525856.MfDSSsiP5d@wuerfel>
On 2016?09?15? 05:33, Arnd Bergmann wrote:
> On Wednesday, September 14, 2016 11:04:33 PM CEST zhichang.yuan wrote:
>> The 8250_hisi_lpc.c support both ACPI and dts similar to 8250_dw :
>>
>> +static struct platform_driver hs_lpc8250_driver = {
>> + .driver = {
>> + .name = "hisi-lpc-uart",
>> + .of_match_table = hs8250_of_match,
>> + .acpi_match_table = ACPI_PTR(hs8250_acpi_match),
>>
>> So, I am a little confused why we need to support dts in 8250_of.c and support ACPI in another
>> driver file.
>>
>
> After looking again, I'm pretty sure that drivers/tty/serial/8250/8250_pnp.c
> will handle the ACPI case without modifications, you just need to adapt
> the 8250_of driver to handle IORESOURCE_IO ports. This will be required
> anyway with the next version once the LPC bridge is on the PCI bus.
>
Yes. You are right.
When the indirectIO patch is accepted, no much changes needed in these two file to support LPC UART.
BTW, what is your target to change these two files to support LPC uart?
Do you think we don't need to add many new c files??
Best,
Zhichang
> Arnd
>
^ permalink raw reply
* [PATCH] clk: mvebu: Add clk support for the orion5x SoC mv88f5181
From: Gregory CLEMENT @ 2016-09-21 10:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160920195919.GD8319@codeaurora.org>
Hi Stephen,
On mar., sept. 20 2016, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 09/20, Gregory CLEMENT wrote:
>> From: Jamie Lentin <jm@lentin.co.uk>
>>
>> Referring to the u-boot sources for the Netgear WNR854T, add support
>> for the mv88f5181.
>>
>> [gregory.clement at free-electrons.com: fix commit title]
>> Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>>
>> Hi Stephen and Mike,
>>
>> do you agree to give your acked-by on this patch. It is part of a
>> convertion of old orion5x Socv to the device tree. If you acked-by
>> this one, then I will be able to take it in my tree and avoiding
>> breaking the git bisect.
>
> Is the problem that we're changing some dts files somewhere and
> those platforms would stop booting if this change wasn't present?
> Given that we're adding a new compatible it seems like we're
> adding new SoC support, so having the clk patch and the dts patch
> come together in -next via a merge instead of basing the dts
> patch on top of the clk patch would be how things are normally
> done.
The problem appear if the dts for the board using the clock driver is
merged in Linus tree before the support in the driver. At this point the
board won't be able to boot.
It introduces a hole in the git bisect between the merge of the clock
subsystem and the arm-soc subsystem.
>
> If we're really changing some dts to be backwards incompatible,
> then I understand the bisect problem and you can have my ack.
>
> Acked-by: Stephen Boyd <sboyd@codeaurora.org.
Thanks,
Gregory
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH] clk: nxp: clk-lpc32xx: Unmap region obtained by of_iomap
From: Vladimir Zapolskiy @ 2016-09-21 10:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474368846-15916-1-git-send-email-arvind.yadav.cs@gmail.com>
On 20.09.2016 13:54, Arvind Yadav wrote:
> From: Arvind Yadav <arvind.yadav.cs@gmail.com>
>
> Free memory mapping, if lpc32xx_clk_init is not successful.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> drivers/clk/nxp/clk-lpc32xx.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
> index 90d740a..34c9735 100644
> --- a/drivers/clk/nxp/clk-lpc32xx.c
> +++ b/drivers/clk/nxp/clk-lpc32xx.c
> @@ -1513,6 +1513,7 @@ static void __init lpc32xx_clk_init(struct device_node *np)
> if (IS_ERR(clk_regmap)) {
> pr_err("failed to regmap system control block: %ld\n",
> PTR_ERR(clk_regmap));
> + iounmap(base);
> return;
> }
>
>
Acked-by: Vladimir Zapolskiy <vz@mleia.com>
Thank you for the patch.
--
With best wishes,
Vladimir
^ permalink raw reply
* [GIT PULL] ARM: mvebu: drivers for v4.9 (#1)
From: Gregory CLEMENT @ 2016-09-21 10:18 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Here is the second tentative of first pull request for drivers for
mvebu for v4.9.
This time both patches received their Acked-by.
Gregory
The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc:
Linux 4.8-rc1 (2016-08-07 18:18:00 -0700)
are available in the git repository at:
git://git.infradead.org/linux-mvebu.git tags/mvebu-drivers-4.9-1
for you to fetch changes up to c336dc7dac0747f6a4901bcd5a497e1b3fb265d7:
pinctrl: mvebu: orion5x: Generalise mv88f5181l support for 88f5181 (2016-09-21 11:49:18 +0200)
----------------------------------------------------------------
mvebu drivers for 4.9 (part 1)
- Add pinctrl and clk support for the Orion5x SoC mv88f5181 variant
----------------------------------------------------------------
Jamie Lentin (2):
clk: mvebu: Add clk support for the orion5x SoC mv88f5181
pinctrl: mvebu: orion5x: Generalise mv88f5181l support for 88f5181
.../devicetree/bindings/clock/mvebu-core-clock.txt | 1 +
.../bindings/pinctrl/marvell,orion-pinctrl.txt | 4 +-
drivers/clk/mvebu/orion.c | 70 ++++++++++++++++++++++
drivers/pinctrl/mvebu/pinctrl-orion.c | 23 +++----
4 files changed, 86 insertions(+), 12 deletions(-)
^ permalink raw reply
* [GIT PULL] ARM: mvebu: soc for v4.9 (#1)
From: Gregory CLEMENT @ 2016-09-21 10:19 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Here is the second tentative of first pull request for soc for mvebu
for v4.9.
The PCI related patch have been discarded.
Gregory
The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc:
Linux 4.8-rc1 (2016-08-07 18:18:00 -0700)
are available in the git repository at:
git://git.infradead.org/linux-mvebu.git tags/mvebu-soc-4.9-1
for you to fetch changes up to 77a1c68a96f4e018860033f3827602014a4f1663:
ARM: orion5x: remove extraneous NO_IRQ (2016-09-14 16:36:20 +0200)
----------------------------------------------------------------
mvebu soc for 4.9 (part 1)
- irq cleanup for old mvebu SoC
----------------------------------------------------------------
Arnd Bergmann (4):
ARM: mv78xx0: simplify ethernet device creation
ARM: mvebu/orion: remove NO_IRQ check from device init
ARM: orion: simplify orion_ge00_switch_init
ARM: orion5x: remove extraneous NO_IRQ
arch/arm/mach-mv78xx0/common.c | 9 +--
arch/arm/mach-orion5x/common.c | 4 +-
arch/arm/mach-orion5x/common.h | 2 +-
arch/arm/mach-orion5x/rd88f5181l-fxo-setup.c | 4 +-
arch/arm/mach-orion5x/rd88f5181l-ge-setup.c | 5 +-
arch/arm/mach-orion5x/rd88f6183ap-ge-setup.c | 6 +-
arch/arm/mach-orion5x/wnr854t-setup.c | 4 +-
arch/arm/mach-orion5x/wrt350n-v2-setup.c | 4 +-
arch/arm/plat-orion/common.c | 85 +++++++++++-----------------
arch/arm/plat-orion/include/plat/common.h | 10 +---
10 files changed, 51 insertions(+), 82 deletions(-)
^ permalink raw reply
* [PATCH] KVM: arm/arm64: timer: Fix hw sync for user space irqchip path
From: Christoffer Dall @ 2016-09-21 10:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57E171AF.1080903@arm.com>
On Tue, Sep 20, 2016 at 06:28:15PM +0100, Marc Zyngier wrote:
> Alex,
>
> On 16/09/16 06:16, Alexander Graf wrote:
> > While adding the new vgic implementation, apparently nobody tested
> > the non-vgic path where user space controls the vgic, so two functions
> > slipped through the cracks that get called in generic code but don't
> > check whether hardware support is enabled.
> >
> > This patch guards them with proper checks to ensure we only try to
> > use vgic data structures if they are available. Without this, I get
> > a stack trace:
> >
> > [ 74.363037] Unable to handle kernel paging request at virtual address ffffffffffffffe8
> > [...]
> > [ 74.929654] [<ffff000008824bcc>] _raw_spin_lock+0x1c/0x58
> > [ 74.935133] [<ffff0000080b7f20>] kvm_vgic_flush_hwstate+0x88/0x288
> > [ 74.941406] [<ffff0000080ab0b4>] kvm_arch_vcpu_ioctl_run+0xfc/0x630
> > [ 74.947766] [<ffff0000080a15bc>] kvm_vcpu_ioctl+0x2f4/0x710
> > [ 74.953420] [<ffff0000082788a8>] do_vfs_ioctl+0xb0/0x728
> > [ 74.958807] [<ffff000008278fb4>] SyS_ioctl+0x94/0xa8
> > [ 74.963844] [<ffff000008083744>] el0_svc_naked+0x38/0x3c
> >
> > Fixes: 0919e84c0
> > Cc: stable at vger.kernel.org
> > Signed-off-by: Alexander Graf <agraf@suse.de>
> > ---
> > virt/kvm/arm/vgic/vgic.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
> > index e83b7fe..9f312ba 100644
> > --- a/virt/kvm/arm/vgic/vgic.c
> > +++ b/virt/kvm/arm/vgic/vgic.c
> > @@ -645,6 +645,9 @@ next:
> > /* Sync back the hardware VGIC state into our emulation after a guest's run. */
> > void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
> > {
> > + if (!vcpu->kvm->arch.vgic.enabled)
> > + return;
> > +
> > vgic_process_maintenance_interrupt(vcpu);
> > vgic_fold_lr_state(vcpu);
> > vgic_prune_ap_list(vcpu);
> > @@ -653,6 +656,9 @@ void kvm_vgic_sync_hwstate(struct kvm_vcpu *vcpu)
> > /* Flush our emulation state into the GIC hardware before entering the guest. */
> > void kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
> > {
> > + if (!vcpu->kvm->arch.vgic.enabled)
> > + return;
> > +
this is not correct, because it checks if the distributor is enabled,
not if the vgic as a thing in KVM is enabled. (The distributor can be
disabled, but a VCPU should still be able to EOI an active interrupt,
for example).
So this check should be
if (!vgic_initialized(vcpu->kvm))
return;
> > spin_lock(&vcpu->arch.vgic_cpu.ap_list_lock);
> > vgic_flush_lr_state(vcpu);
> > spin_unlock(&vcpu->arch.vgic_cpu.ap_list_lock);
> >
>
> I hate that fix, because it papers over the fact that we have uninitialized
> structures all over the shop, and that's not exactly great.
I'm not completely convinced about this, because we have
vgic_initialized() checks in the arch timer code as well, and I can't
easily figure out if initializing all data structures etc. to a shim
would work for all vgic interactions.
Basically, I think we have a choice between
(1) locate *all* entry points to the gic code, and make sure they're
guarded with vgic_initialized(), or
(2) do something like you suggest and still go through all interactions
between the vgic and the rest of the system and ensure that whatever
shim/empty data structures we've allocated, actually end up doing the
right thing.
My gut feeling is to lean towards (1), but I don't feel overly strongly
about that.
FWIW: A few comments on the patch below:
>
> How about the following instead:
>
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index c94b90d..0961128 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -472,6 +472,9 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
> return ret;
> }
>
> + if (unlikely(!irqchip_in_kernel(kvm)))
> + kvm_no_vgic_init(kvm);
> +
> /*
> * Enable the arch timers only if we have an in-kernel VGIC
> * and it has been properly initialized, since we cannot handle
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index bb46c03..1b70b1e 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -327,4 +327,6 @@ int kvm_send_userspace_msi(struct kvm *kvm, struct kvm_msi *msi);
> */
> int kvm_vgic_setup_default_irq_routing(struct kvm *kvm);
>
> +void kvm_no_vgic_init(struct kvm *kvm);
> +
> #endif /* __KVM_ARM_VGIC_H */
> diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
> index 83777c1..7b8f12b 100644
> --- a/virt/kvm/arm/vgic/vgic-init.c
> +++ b/virt/kvm/arm/vgic/vgic-init.c
> @@ -151,9 +151,11 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
> INIT_LIST_HEAD(&dist->lpi_list_head);
> spin_lock_init(&dist->lpi_list_lock);
>
> - dist->spis = kcalloc(nr_spis, sizeof(struct vgic_irq), GFP_KERNEL);
> - if (!dist->spis)
> - return -ENOMEM;
> + if (nr_spis) {
> + dist->spis = kcalloc(nr_spis, sizeof(struct vgic_irq), GFP_KERNEL);
> + if (!dist->spis)
> + return -ENOMEM;
> + }
Don't we still end up with dist->spis pointing to nothing if there
hasn't been an init and nr_spis == 0 ?
Perhaps that's not a problem, but I don't easily understand which parts
of the vgic structure we must initialize and which we don't.
>
> /*
> * In the following code we do not take the irq struct lock since
> @@ -325,6 +327,21 @@ int vgic_lazy_init(struct kvm *kvm)
> return ret;
> }
>
> +void kvm_no_vgic_init(struct kvm *kvm)
> +{
> + mutex_lock(&kvm->lock);
> + if (unlikely(!vgic_initialized(kvm))) {
> + struct kvm_vcpu *vcpu;
> + int i;
> +
> + kvm_vgic_dist_init(kvm, 0);
I think you need to check the return value here, unless the rationale is
that when passing 0 as the second argument, it cannot fail. That's a
pretty brittle construct though, IMHO.
> + kvm_for_each_vcpu(i, vcpu, kvm)
> + kvm_vgic_vcpu_init(vcpu);
> + kvm->arch.vgic.initialized = true;
> + }
> + mutex_unlock(&kvm->lock);
> +}
> +
> /* RESOURCE MAPPING */
>
> /**
>
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH v8 1/4] soc: mediatek: Refine scpsys to support multiple platform
From: Matthias Brugger @ 2016-09-21 10:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1468911883-41882-2-git-send-email-jamesjj.liao@mediatek.com>
On 19/07/16 09:04, 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 837effe..1f3555a 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]);
Should be:
for (i = CLK_NONE + 1; i < CLK_MAX; i++)
then we can get rid of clk_ids[].
> +}
> +
> +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);
> +
While at it, please leave this block of code between
devm_ioremap_resource(...) and syscon_regmap_lookup_by_phandle(...)
Thanks,
Matthias
^ permalink raw reply
* [PATCH v2 0/9] dmaengine: ti drivers: enable COMPILE_TESTing
From: Peter Ujfalusi @ 2016-09-21 10:26 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Changes since v1:
- added the compiler warning message to ti-dma-crossbar enum type patch
- moved the Kconfig patches at the end of the seris
The following series will enable unconditional COMPILE_TEST coverage for the
following drivers: omap-dma, edma and ti-dma-crossbar
The series includes fixes noticed when compiling the drivers for x86_64 and
aarch64.
Replaces the patch Vinod sent to enable partial compile testing for omap-dma:
http://www.spinics.net/lists/dmaengine/msg11025.html
Regards,
Peter
---
Peter Ujfalusi (9):
dmaengine: edma: Add missing MODULE_DEVICE_TABLE() for of_device_id
structs
dmaengine: edma: Use enum for eDMA binding type (legacy vs TPCC)
dmaengine: edma: Use correct type for of_find_property() third
parameter
dmaengine/ARM: omap-dma: Fix the DMAengine compile test on non OMAP
configs
dmaengine: ti-dma-crossbar: Correct type for of_find_property() third
parameter
dmaengine: ti-dma-crossbar: Use enum for crossbar type
dmaengine: edma: enable COMPILE_TEST
dmaengine: omap-dma: enable COMPILE_TEST
dmaengine: ti-dma-crossbar: enable COMPILE_TEST
drivers/dma/Kconfig | 8 ++++----
drivers/dma/edma.c | 15 ++++++++++-----
drivers/dma/ti-dma-crossbar.c | 12 +++++++-----
include/linux/omap-dma.h | 19 +++++++++++++++++++
4 files changed, 40 insertions(+), 14 deletions(-)
--
2.10.0
^ permalink raw reply
* [PATCH v2 1/9] dmaengine: edma: Add missing MODULE_DEVICE_TABLE() for of_device_id structs
From: Peter Ujfalusi @ 2016-09-21 10:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160921102637.24845-1-peter.ujfalusi@ti.com>
The MODULE_DEVICE_TABLE() were missing from the driver for the of_device_id
structures.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
drivers/dma/edma.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 3d277fa76c1a..c2098a4b4dcf 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -274,11 +274,13 @@ static const struct of_device_id edma_of_ids[] = {
},
{}
};
+MODULE_DEVICE_TABLE(of, edma_of_ids);
static const struct of_device_id edma_tptc_of_ids[] = {
{ .compatible = "ti,edma3-tptc", },
{}
};
+MODULE_DEVICE_TABLE(of, edma_tptc_of_ids);
static inline unsigned int edma_read(struct edma_cc *ecc, int offset)
{
--
2.10.0
^ permalink raw reply related
* [PATCH v2 2/9] dmaengine: edma: Use enum for eDMA binding type (legacy vs TPCC)
From: Peter Ujfalusi @ 2016-09-21 10:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160921102637.24845-1-peter.ujfalusi@ti.com>
Fixes the following warning when compiling the driver for 64bit
architectures (x86_64 for example):
drivers/dma/edma.c:2185:16: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
if (match && (u32)match->data == EDMA_BINDING_TPCC)
^
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
drivers/dma/edma.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index c2098a4b4dcf..4c8818278fcc 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -261,8 +261,11 @@ static const struct edmacc_param dummy_paramset = {
.ccnt = 1,
};
-#define EDMA_BINDING_LEGACY 0
-#define EDMA_BINDING_TPCC 1
+enum edma_binding_type {
+ EDMA_BINDING_LEGACY = 0,
+ EDMA_BINDING_TPCC,
+};
+
static const struct of_device_id edma_of_ids[] = {
{
.compatible = "ti,edma3",
@@ -2184,7 +2187,8 @@ static int edma_probe(struct platform_device *pdev)
const struct of_device_id *match;
match = of_match_node(edma_of_ids, node);
- if (match && (u32)match->data == EDMA_BINDING_TPCC)
+ if (match &&
+ (enum edma_binding_type)match->data == EDMA_BINDING_TPCC)
legacy_mode = false;
info = edma_setup_info_from_dt(dev, legacy_mode);
--
2.10.0
^ permalink raw reply related
* [PATCH v2 3/9] dmaengine: edma: Use correct type for of_find_property() third parameter
From: Peter Ujfalusi @ 2016-09-21 10:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160921102637.24845-1-peter.ujfalusi@ti.com>
The correct type is int and not for the third parameter of
of_find_property().
Fixes compilation for 64bit architectures (x86_64, aarch64).
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
drivers/dma/edma.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 4c8818278fcc..16c0bb92fa4d 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -2024,8 +2024,7 @@ static struct edma_soc_info *edma_setup_info_from_dt(struct device *dev,
{
struct edma_soc_info *info;
struct property *prop;
- size_t sz;
- int ret;
+ int sz, ret;
info = devm_kzalloc(dev, sizeof(struct edma_soc_info), GFP_KERNEL);
if (!info)
--
2.10.0
^ permalink raw reply related
* [PATCH v2 4/9] dmaengine/ARM: omap-dma: Fix the DMAengine compile test on non OMAP configs
From: Peter Ujfalusi @ 2016-09-21 10:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160921102637.24845-1-peter.ujfalusi@ti.com>
The DMAengine driver for omap-dma use three function calls from the
plat-omap legacy driver. When the DMAengine driver is built when ARCH_OMAP
is not set, the compilation will fail due to missing symbols.
Add empty inline functions to allow the DMAengine driver to be compiled
with COMPILE_TEST.
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
include/linux/omap-dma.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/include/linux/omap-dma.h b/include/linux/omap-dma.h
index 1d99b61adc65..290081620b3e 100644
--- a/include/linux/omap-dma.h
+++ b/include/linux/omap-dma.h
@@ -297,6 +297,7 @@ struct omap_system_dma_plat_info {
#define dma_omap15xx() __dma_omap15xx(d)
#define dma_omap16xx() __dma_omap16xx(d)
+#if defined(CONFIG_ARCH_OMAP)
extern struct omap_system_dma_plat_info *omap_get_plat_info(void);
extern void omap_set_dma_priority(int lch, int dst_port, int priority);
@@ -355,4 +356,22 @@ static inline int omap_lcd_dma_running(void)
}
#endif
+#else /* CONFIG_ARCH_OMAP */
+
+static inline struct omap_system_dma_plat_info *omap_get_plat_info(void)
+{
+ return NULL;
+}
+
+static inline int omap_request_dma(int dev_id, const char *dev_name,
+ void (*callback)(int lch, u16 ch_status, void *data),
+ void *data, int *dma_ch)
+{
+ return -ENODEV;
+}
+
+static inline void omap_free_dma(int ch) { }
+
+#endif /* CONFIG_ARCH_OMAP */
+
#endif /* __LINUX_OMAP_DMA_H */
--
2.10.0
^ permalink raw reply related
* [PATCH v2 5/9] dmaengine: ti-dma-crossbar: Correct type for of_find_property() third parameter
From: Peter Ujfalusi @ 2016-09-21 10:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160921102637.24845-1-peter.ujfalusi@ti.com>
The correct type is int and not for the third parameter of
of_find_property().
Fixes compilation for 64bit architectures (x86_64, aarch64).
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
drivers/dma/ti-dma-crossbar.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/ti-dma-crossbar.c b/drivers/dma/ti-dma-crossbar.c
index 5ae294b256a7..e4f3bd1ae264 100644
--- a/drivers/dma/ti-dma-crossbar.c
+++ b/drivers/dma/ti-dma-crossbar.c
@@ -311,7 +311,7 @@ static int ti_dra7_xbar_probe(struct platform_device *pdev)
struct property *prop;
struct resource *res;
u32 safe_val;
- size_t sz;
+ int sz;
void __iomem *iomem;
int i, ret;
--
2.10.0
^ permalink raw reply related
* [PATCH v2 6/9] dmaengine: ti-dma-crossbar: Use enum for crossbar type
From: Peter Ujfalusi @ 2016-09-21 10:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160921102637.24845-1-peter.ujfalusi@ti.com>
Fixes compiler warnings on 64bit architectures:
drivers/dma/ti-dma-crossbar.c: In function ?ti_dra7_xbar_probe?:
drivers/dma/ti-dma-crossbar.c:398:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
xbar->dma_offset = (u32)match->data;
^
drivers/dma/ti-dma-crossbar.c: In function ?ti_dma_xbar_probe?:
drivers/dma/ti-dma-crossbar.c:431:10: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
switch ((u32)match->data) {
^
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
drivers/dma/ti-dma-crossbar.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/ti-dma-crossbar.c b/drivers/dma/ti-dma-crossbar.c
index e4f3bd1ae264..876e4ccaf033 100644
--- a/drivers/dma/ti-dma-crossbar.c
+++ b/drivers/dma/ti-dma-crossbar.c
@@ -16,8 +16,10 @@
#include <linux/of_device.h>
#include <linux/of_dma.h>
-#define TI_XBAR_DRA7 0
-#define TI_XBAR_AM335X 1
+enum ti_xbar_type {
+ TI_XBAR_DRA7 = 0,
+ TI_XBAR_AM335X,
+};
static const struct of_device_id ti_dma_xbar_match[] = {
{
@@ -395,7 +397,7 @@ static int ti_dra7_xbar_probe(struct platform_device *pdev)
xbar->dmarouter.dev = &pdev->dev;
xbar->dmarouter.route_free = ti_dra7_xbar_free;
- xbar->dma_offset = (u32)match->data;
+ xbar->dma_offset = (enum ti_xbar_type)match->data;
mutex_init(&xbar->mutex);
platform_set_drvdata(pdev, xbar);
@@ -428,7 +430,7 @@ static int ti_dma_xbar_probe(struct platform_device *pdev)
if (unlikely(!match))
return -EINVAL;
- switch ((u32)match->data) {
+ switch ((enum ti_xbar_type)match->data) {
case TI_XBAR_DRA7:
ret = ti_dra7_xbar_probe(pdev);
break;
--
2.10.0
^ permalink raw reply related
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