* [RFC] ARM: memory: da8xx-ddrctl: new driver
From: Mark Rutland @ 2016-10-24 17:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7hd1ipzm3h.fsf@baylibre.com>
On Mon, Oct 24, 2016 at 10:35:30AM -0700, Kevin Hilman wrote:
> Hi Mark,
>
> Mark Rutland <mark.rutland@arm.com> writes:
> > On Mon, Oct 24, 2016 at 06:46:36PM +0200, Bartosz Golaszewski wrote:
> >> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
> >> +{
> >> + const struct da8xx_ddrctl_config_knob *knob;
> >> + const struct da8xx_ddrctl_setting *setting;
> >> + u32 regprop[2], base, memsize, reg;
> >> + struct device_node *node, *parent;
> >> + void __iomem *ddrctl;
> >> + const char *board;
> >> + struct device *dev;
> >> + int ret;
> >> +
> >> + dev = &pdev->dev;
> >> + node = dev->of_node;
> >> +
> >> + /* Find the board name. */
> >> + for (parent = node;
> >> + !of_node_is_root(parent);
> >> + parent = of_get_parent(parent));
> >> +
> >> + ret = of_property_read_string(parent, "compatible", &board);
> >> + if (ret) {
> >> + dev_err(dev, "unable to read the soc model\n");
> >> + return ret;
> >> + }
> >
> > I can see that you want to expose sysfs knobs for this, but is it really
> > necessary to match boards like this? It's very fragile, and commits us
> > to maintaining a database of board data (i.e. a board file).
> >
> > I am very much not keen on that.
>
> The original proposal[1] was to create DT properties reflecting the
> various knobs in the DDR Controller, but that was frowned upon since
> that was more HW configuration than hardware description.
>
> That resulted in this approach which keeps the HW configuration values
> in the driver, and selectable based on DT compatible.
>
> IMO, neither aproach is pretty. From a DT maintainer perspective, can
> you comment on your preference?
>From my PoV, it really depends on *why* we need this. What does the
tuning gain us, and is it specific to a given use-case?
Thanks,
Mark.
^ permalink raw reply
* [PATCH v3 3/8] PM / Domains: Allow domain power states to be read from DT
From: Sudeep Holla @ 2016-10-24 17:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164846.GE72940@linaro.org>
On 24/10/16 17:48, Lina Iyer wrote:
> Hi Sudeep,
[..]
>
> I was able to find the original thread, where we discussed this [1].
>
> [1]. http://www.serverphorums.com/read.php?12,1303996
>
I went through the discussion and that's valid discussion and I agree.
But what am looking for is backward compatibility and not exactly same
as the discussion and I agree that can be solved in different way.
I just feel that's one easily way to solve such issues. I am open to
suggestions.
--
Regards,
Sudeep
^ permalink raw reply
* Add Allwinner Q8 tablets hardware manager
From: Mark Rutland @ 2016-10-24 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161014075337.10452-1-hdegoede@redhat.com>
On Fri, Oct 14, 2016 at 09:53:31AM +0200, Hans de Goede wrote:
> Hi Rob, Mark, et al.,
Hi Hans,
Apologies for the delay in replying to this.
I'd like to be clear that I do understand that there is a problem that
needs to be addressed here. However, I do not believe that the *current*
in-kernel approach is correct. More on that below.
> Mark, I know that we discussed this at ELCE and you clearly indicated
> that according to you this does not belong in the kernel. I was a bit
> surprised by this part of the discussion.
>
> I had posted a RFC earlier and Rob had indicated that given that the q8
> tablets are a special case, as my code uses actual probing rather then some
> pre-arranged id mechanism with say an eeprom, that doing this in a
> non-generic manner would be ok for my special case.
To some extent, Rob and I may have differing views here; I'm not
entirely sure what Rob's view is, and I cannot talk on his behalf. I
certainly must apologise for having not commented on said RFC, however.
> So on to why I believe that the kernel is the best place to do this, at least
> for my special use-case:
>
> 1. Configurability
>
> Since the q8 tablets do not have any id mechanism I'm probing i2c busses to
> generate i2c client dt nodes with the right address and compatible.
> Some info in these nodes (e.g. touchscreen firmware-name) is not probable at
> all and gets set by heuristics. This heuristics may get things wrong.
> So my current implementation offers kernel cmdline options to override this.
As I mentioned at ELCE, one major concern I have with this approach is
this probing, which in part relies on a collection of heuristics.
I'm worried that this is very fragile, and sets us up for having to
maintain a much larger collection of heuristics and/or a database of
particular boards in future. This is fragile, defeats much of the gain
from DT.
Worse, this could be completely incompatible with some arbitrary board
that comes by in future, because the kernel assumed something that was
not true, which it would not have done if things were explicitly
described to the kernel.
As I mentioned at ELCE, I'm not opposed to the concept of the kernel
applying overlays or fixups based on a well-defined set of criteria;
having some of that embedded in the DT itself would be remarkably
helpful. However, I am very much not keen on loosely defined criteria as
with here, as it couples the DT and kernel, and creates problems longer
term, as described above.
> Although having to specify kernel cmdline options is not the plug and play
> experience I want to give end-users most distros do have documentation on
> how to do this and doing this is relatively easy for end-users. Moving this
> to the bootloader means moving to some bootloader specific config mechanism
> which is going to be quite hard (and possibly dangerous) for users to use.
I have to ask, why is it more dangerous?
Perhaps more difficult, but that can be solved, if the manual
corrections are simply a set of options to be passed to the kernel, I
don't see why the bootloader cannot pick this up.
> 2. Upgradability
>
> Most users treat the bootloader like they treat an x86 machine BIOS/EFI,
> they will never upgrade it unless they really have to. This means that it
> will be very difficult to get users to actual benefit from bug-fixes /
> improvements done to the probing code. Where as the kernel on boards running
> e.g. Debian or Fedora gets regular updates together with the rest of the
> system.
Given that DTBs are supposed to remain supported, users should find
themselves with a system that continues to work, but may not have all
the bells and whistles it could, much like elsewhere.
While it's true that we have issues in this area, I don't think that's
an argument for putting things into the kernel for this specific set of
boards.
> 3. Infrastructure
>
> The kernel simply has better infrastructure for doing these kind of things.
At least on the DT front, a lot of work has gone into improving the
infrastructure, e.g. the work that Free Electrons have done [1]. I
appreciate that the infrastructure for things like poking SPI may not be
as advanced.
Which bits of infrastructure do you find lacking today?
> Yes we could improve the bootloader, but the kernel is also improving
> and likely at a faster rate. Besides that the purpose of the
> bootloader is mostly to be simple and small, load the kernel and get
> out of the way, not to be a general purpose os kernel. So it will
> simply always have less infrastructure and that is a good thing,
> otherwise we will be writing another general purpose os which is a
> waste of effort.
I think this conflates a number of details. Yes, we'd like firmware and
bootloaders to be small, and yes, their infrastructure and feature
support will be smaller than a general purpose OS.
That doesn't imply that they cannot have features necessary to boostrap
an OS.
It's also not strictly necessary that the firmware or bootloader have
the capability to do all this probing, as that could be contained in
another part (e.g. a U-Boot application which is run once to detect the
board details, logging this into a file).
It's also possible to ship an intermediary stage (e.g. like the
impedance matcher) which can be upgradeable independently of the kernel.
> 4. This is not a new board file
>
> Mark, at ELCE I got the feeling that your biggest worry / objection is
> that this will bring back board files, but that is not the case, if you
> look at the actual code it is nothing like a board-file at all. Where a
> board file was instantiating device after device after device from c-code,
> with maybe a few if-s in there to deal with board revisions. This code is
> all about figuring out which accelerometer and touchscreen there are,
> to then add nodes to the dt for this 2 devices, almost all the code is
> probing, the actual dt modifying code is tiny and no direct device
> instantiation is done at all.
Sorry, but I must disagree. This code:
(a) identifies a set of boards based on a top-level compatible string.
i.e. it's sole purpose is to handle those boards.
(b) assumes non-general properties of those boards (e.g. that poking
certain SPI endpoints is safe).
(c) applies arbitrary properties to the DT, applying in-built knowledge
of those boards (in addition to deep structural knowledge of the
DTB in question).
To me, given the implicit knowledge, that qualifies as a "board file".
As I mentioned at ELCE, if this had no knowledge of the boards in
question, I would be less concerned. e.g. if there was a well-defined
identification mechanism, describe in the DT, with fixups also defined
in the DT.
> 5. This is an exception, not the rule
>
> Yes this is introducing board (family of boards) specific c-code into the
> kernel, so in a way it is reminiscent of board files. But sometimes this is
> necessary, just look at all the vendor / platform specific code in the kernel
> in drivers/platform/x86, or all the places where DMI strings are used to
> uniquely identify a x86 board and adjust behavior.
>
> But this really is the exception, not the rule. I've written/upstreamed a
> number of drivers for q8 tablets hardware and if you look at e.g. the
> silead touchscreen driver then in linux-next this is already used for 5
> ARM dts files where no board specific C-code is involved at all.
>
> So this again is very different from the board file era, where C-code
> had to be used to fill device specific platform-data structs, here all
> necessary info is contained in the dt-binding and there are many users
> who do not need any board specific C-code in the kernel at all.
>
> So dt is working as it should and is avoiding board specific C-code for
> the majority of the cases. But sometimes hardware is not as we ideally
> would like it to be; and for those *exceptions* we are sometimes going
> to need C-code in the kernel, just like there is "board" specific C-code
> in the x86 code.
Your talk convinced me that we're both going to see more variation
within this family of boards, and that we'll see more families of boards
following a similar patter. Given that, I think that we need a more
general solution, as I commented on the RFC.
That doesn't necessarily mean that this can't happen in the kernel, but
it certainly needs to be more strictly defined, e.g. with match criteria
and fixups explicit in the DTB.
Thanks,
Mark.
[1] http://free-electrons.com/blog/dt-overlay-uboot-libfdt/
[2] https://github.com/zonque/pxa-impedance-matcher
^ permalink raw reply
* [PATCH 00/10] arm64: move thread_info off of the task stack
From: Laura Abbott @ 2016-10-24 17:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476904234-9511-1-git-send-email-mark.rutland@arm.com>
On 10/19/2016 12:10 PM, Mark Rutland wrote:
> Hi all,
>
> Building atop of Andy's work on x86 and generic code, these patches move
> arm64's thread_info off of the stack and into task_struct. This protects
> thread_info from corruption in the face of stack overflow, and serves as
> a step towards fully robust stack overflow handling, which will be
> addressed by subsequent patches.
>
> These patches are based atop of a preparatory series [1] (itself based
> on v4.9-rc1) that's also necessary for s390. I've placed those patches
> in a branch [2] on my kernel.org repo, along with this series [3]. I'm
> hoping that the prep work will be able to become a stable branch/tag
> soon.
>
> I've given the series some light testing on a couple of SMP arm64
> platforms, but this has yet to see a thorough beating; please do try to
> make this fall over!
>
> Since RFC [4]:
> * Rely on prior patches to make thread_info arch-specific
> * Make smp_processor_id() use a per-cpu variable
> * Split out current_stack_pointer
> * Make SMP actually work
>
> [1] http://lkml.kernel.org/r/1476901693-8492-1-git-send-email-mark.rutland at arm.com
> [2] https://git.kernel.org/cgit/linux/kernel/git/mark/linux.git/log/?h=core/ti-stack-split
> [3] https://git.kernel.org/cgit/linux/kernel/git/mark/linux.git/log/?h=arm64/ti-stack-split
> [4] http://lkml.kernel.org/r/1473947349-14521-1-git-send-email-mark.rutland at arm.com
>
> Thanks,
> Mark.
>
> Mark Rutland (10):
> arm64: thread_info remove stale items
> arm64: asm-offsets: remove unused definitions
> arm64: factor out current_stack_pointer
> arm64: traps: simplify die() and __die()
> arm64: prep stack walkers for THREAD_INFO_IN_TASK
> arm64: move sp_el0 and tpidr_el1 into cpu_suspend_ctx
> arm64: smp: prepare for smp_processor_id() rework
> arm64: make cpu number a percpu variable
> arm64: assembler: introduce ldr_this_cpu
> arm64: split thread_info from task stack
>
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/Kbuild | 1 -
> arch/arm64/include/asm/assembler.h | 19 +++++++++++++++----
> arch/arm64/include/asm/current.h | 22 ++++++++++++++++++++++
> arch/arm64/include/asm/percpu.h | 2 ++
> arch/arm64/include/asm/perf_event.h | 2 ++
> arch/arm64/include/asm/smp.h | 7 ++++++-
> arch/arm64/include/asm/stack_pointer.h | 9 +++++++++
> arch/arm64/include/asm/suspend.h | 2 +-
> arch/arm64/include/asm/thread_info.h | 32 +-------------------------------
> arch/arm64/kernel/asm-offsets.c | 3 +--
> arch/arm64/kernel/entry.S | 6 +++---
> arch/arm64/kernel/head.S | 11 ++++++-----
> arch/arm64/kernel/process.c | 33 +++++++++++++++++++++++++++------
> arch/arm64/kernel/return_address.c | 1 +
> arch/arm64/kernel/sleep.S | 3 ---
> arch/arm64/kernel/smp.c | 14 +++++++++++---
> arch/arm64/kernel/stacktrace.c | 6 ++++++
> arch/arm64/kernel/suspend.c | 6 ------
> arch/arm64/kernel/traps.c | 14 +++++++-------
> arch/arm64/mm/proc.S | 6 ++++++
> 21 files changed, 127 insertions(+), 73 deletions(-)
> create mode 100644 arch/arm64/include/asm/current.h
> create mode 100644 arch/arm64/include/asm/stack_pointer.h
>
I pulled the arm64/ti-stack-split branch on top of a Fedora
tree and ran back-to-back kernel RPM builds for a long weekend.
It's still going as of this morning so you can take that as a
Tested-by: Laura Abbott <labbott@redhat.com>
Thanks,
Laura
^ permalink raw reply
* [RFC] ARM: memory: da8xx-ddrctl: new driver
From: Kevin Hilman @ 2016-10-24 17:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024170035.GO15620@leverpostej>
Hi Mark,
Mark Rutland <mark.rutland@arm.com> writes:
> On Mon, Oct 24, 2016 at 06:46:36PM +0200, Bartosz Golaszewski wrote:
>> Create a new driver for the da8xx DDR2/mDDR controller and implement
>> support for writing to the Peripheral Bus Burst Priority Register.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>> .../memory-controllers/ti-da8xx-ddrctl.txt | 20 +++
>> drivers/memory/Kconfig | 8 +
>> drivers/memory/Makefile | 1 +
>> drivers/memory/da8xx-ddrctl.c | 187 +++++++++++++++++++++
>> 4 files changed, 216 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
>> create mode 100644 drivers/memory/da8xx-ddrctl.c
>>
>> diff --git
>> a/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
>> b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
>> new file mode 100644
>> index 0000000..f0eda59
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
>> @@ -0,0 +1,20 @@
>> +* Device tree bindings for Texas Instruments da8xx DDR2/mDDR memory controller
>> +
>> +The DDR2/mDDR memory controller present on Texas Instruments da8xx SoCs memory
>> +maps a set of registers which allow to tweak the controller's behavior.
>
> This is a description of the *driver*. The device itself doesn't map
> some registers, it features them. Please descrive the *device*.
>
>> +
>> +Documentation:
>> +OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
>> +
>> +Required properties:
>> +
>> +- compatible: "ti,da850-ddrctl" - for da850 SoC based boards
>
> Perhaps:
>
> "ti,da850-ddr-controller"
>
>> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
>> +{
>> + const struct da8xx_ddrctl_config_knob *knob;
>> + const struct da8xx_ddrctl_setting *setting;
>> + u32 regprop[2], base, memsize, reg;
>> + struct device_node *node, *parent;
>> + void __iomem *ddrctl;
>> + const char *board;
>> + struct device *dev;
>> + int ret;
>> +
>> + dev = &pdev->dev;
>> + node = dev->of_node;
>> +
>> + /* Find the board name. */
>> + for (parent = node;
>> + !of_node_is_root(parent);
>> + parent = of_get_parent(parent));
>> +
>> + ret = of_property_read_string(parent, "compatible", &board);
>> + if (ret) {
>> + dev_err(dev, "unable to read the soc model\n");
>> + return ret;
>> + }
>
> I can see that you want to expose sysfs knobs for this, but is it really
> necessary to match boards like this? It's very fragile, and commits us
> to maintaining a database of board data (i.e. a board file).
>
> I am very much not keen on that.
The original proposal[1] was to create DT properties reflecting the
various knobs in the DDR Controller, but that was frowned upon since
that was more HW configuration than hardware description.
That resulted in this approach which keeps the HW configuration values
in the driver, and selectable based on DT compatible.
IMO, neither aproach is pretty. From a DT maintainer perspective, can
you comment on your preference?
Thanks,
Kevin
[1] https://marc.info/?l=linux-arm-kernel&m=147672200715076&w=2
^ permalink raw reply
* Question about arm64 earlycon
From: Duc Dang @ 2016-10-24 17:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1f1eeb4a-925a-9fac-d442-fbee503deb0a@arm.com>
On Mon, Oct 24, 2016 at 3:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 24/10/16 11:06, Arnd Bergmann wrote:
>> On Sunday, October 23, 2016 12:26:59 AM CEST Duc Dang wrote:
>>> Hi Catalin, Marc, Mark, Arnd,
>>>
>>> I am testing with 3.12 kernel with earlyprintk enabled and I see some
>>> garbage characters in the console log right before the message
>>> indicating that the real console device is initialized:
>>>
>>> <some garbage characters here>01c020000.serial: ttyS0 at MMIO
>>> 0x1c020000 (irq = 108, base_baud = 3125000) is a U6_16550A
>>> console [ttyS0] enabled, bootconsole disabled
>
> Well, you get two independent, unsynchronized bits of code writing to
> the same peripheral. Things are bound to go badly. Unless you're trying
> to debug things, do not use earlyprintk in production.
This is what I am testing for now: not passing earlyprintk parameter,
and I don't see any weird character.
Is it also true about earlycon? We should not use either earlyprintk
(already gone long time ago) or earlycon for production?
>
>>>
>>> I looked through early_prink.c file and printk.c file and it looks
>>> like there is case that some early boot code can touch the UART
>>> hardware via ealy console driver while the 'real' console driver is
>>> setting up the same UART port? Please let me know if I missed some
>>> important piece of code that can prevent this.
>>
>> I don't think we every supported earlyprintk on arm64, and
>> earlycon support may have been added later.
>
> We did support some form of earlyprintk for a while (though not in the
> same way as 32bit ARM does), until Rob introduced earlycon.
>
>> If you can't use a modern kernel, try backporting all the
>> relevant earlycon changes.
>
> Agreed. That's the most sensible course of action.
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
Regards,
Duc Dang.
^ permalink raw reply
* [PATCH v3 3/8] PM / Domains: Allow domain power states to be read from DT
From: Sudeep Holla @ 2016-10-24 17:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164846.GE72940@linaro.org>
On 24/10/16 17:48, Lina Iyer wrote:
> Hi Sudeep,
>
> On Mon, Oct 24 2016 at 07:39 -0600, Sudeep Holla wrote:
>>
>>
>> On 14/10/16 18:47, Lina Iyer wrote:
>>> This patch allows domains to define idle states in the DT. SoC's can
>>> define domain idle states in DT using the "domain-idle-states" property
>>> of the domain provider. Add API to read the idle states from DT that can
>>> be set in the genpd object.
>>>
>>> This patch is based on the original patch by Marc Titinger.
>>>
>>> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>> ---
>>> drivers/base/power/domain.c | 94
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/pm_domain.h | 8 ++++
>>> 2 files changed, 102 insertions(+)
>>>
>>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>> index 37ab7f1..9af75ba 100644
>>> --- a/drivers/base/power/domain.c
>>> +++ b/drivers/base/power/domain.c
>>> @@ -1916,6 +1916,100 @@ out:
>>> return ret ? -EPROBE_DEFER : 0;
>>> }
>>> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>>> +
>>> +static const struct of_device_id idle_state_match[] = {
>>> + { .compatible = "arm,idle-state", },
>>> + { }
>>> +};
>>> +
>>
>> I still think it's better to have another compatible to serve this
>> purpose. We don't want to end up creating genpd domains just because
>> they are "arm,idle-state" compatible IMO ?
>>
>> I agree you can prevent it checking for OSC mode support in the
>> firmware. But I want to understand if you have any strong reasons for
>> avoiding that approach.
>>
> Why are you still held up with OSI/PC PSCI modes?
I am just pointing that out to make sure you are not defining these
DT bindings with just QCOM platform and OSC in consideration.
I am thinking about how can it be used/extended in other use-cases.
> I repeat again this
> series is not about any of that, it is just about PM domains.
I completely understand that, no argument on that. What I worry is that
if a system(in future) represents this power domains and domain idles
states and doesn't want to create a genpd, do we have to handle it
differently or even the way your CPUidle series would do or can we say
those states need to specify that they are compatible with the new
feature(the one being added in this series) with a new compatible.
I prefer the latter than the former to avoid all possible workarounds
in future.
> PM domains
> have idle states and the idle-state description is similar in definition
> to arm,idle-state and therefore uses the same compatible. There is no
> point re-defining something that already exists in the kernel.
>
Yes I understand that but "arm,idle-states" were not defined with this
feature in mind and hence I am bit worried if it could cause any issue
especially if deprecate cpu-idle-states and move to this model
completely. I really don't like this mix and hence I am raising the
concern here. I am trying to ease that migration.
> I was able to find the original thread, where we discussed this [1].
>
> I suggest, you read about PM domains and its idle states and understand
> this series in the context of PM domains.
>
Sure, will do that. Thanks for pointing that out. But the concern I am
raising is entirely different. I am asking if this re-use will cause any
issue in future as pointed out above.
I re-iterate that I understand this series is independent of the CPUIdle
and hence asking why not make it completely independent by just adding
the new compatible.
I am *not asking to redefine something completely*. What I am saying is
*just* to add new compatible that may(for cpu devices)/may not(for
other/non-CPU devices) be used along with "arm,idle-state".
I may be too paranoid here but I think that's safer. It helps to skip
creating of genpd if required for some domains as I had explained before.
--
Regards,
Sudeep
^ permalink raw reply
* Question about arm64 earlycon
From: Duc Dang @ 2016-10-24 17:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024110904.GF15620@leverpostej>
On Mon, Oct 24, 2016 at 4:09 AM, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Mon, Oct 24, 2016 at 11:17:36AM +0100, Marc Zyngier wrote:
> > On 24/10/16 11:06, Arnd Bergmann wrote:
> > > On Sunday, October 23, 2016 12:26:59 AM CEST Duc Dang wrote:
> > >> Hi Catalin, Marc, Mark, Arnd,
> > >>
> > >> I am testing with 3.12 kernel with earlyprintk enabled and I see some
> > >> garbage characters in the console log right before the message
> > >> indicating that the real console device is initialized:
>
> What exactly are you passing on the command line?
This is what I have:
earlyprintk=uart8250-32bit,0x1c020000
I should be more specific: the early console prints characters just
fine (I see all the early boot log). Only at the moment before
switching to the real console, I occasionally see some garbage
characters.
>
> > >> <some garbage characters here>01c020000.serial: ttyS0 at MMIO
> > >> 0x1c020000 (irq = 108, base_baud = 3125000) is a U6_16550A
> > >> console [ttyS0] enabled, bootconsole disabled
>
> Was the UART already configured by FW? Had the firmware output anything
> at this point?
Yes, UART was already configured by FW (U-Boot) and at this point,
U-Boot is already gone.
>
> Did the firmware's UART rate match that of the kernel? If not, the issue
> might just be that the rate doesn't match; earlycon/earlyprintk won't
> configure that, while the real console will.
The baud-rate does not change between U-Boot and kernel.
>
> Or perhaps we race with some clock configuration...
>
> > >> I looked through early_prink.c file and printk.c file and it looks
> > >> like there is case that some early boot code can touch the UART
> > >> hardware via ealy console driver while the 'real' console driver is
> > >> setting up the same UART port? Please let me know if I missed some
> > >> important piece of code that can prevent this.
> > >
> > > I don't think we every supported earlyprintk on arm64, and
> > > earlycon support may have been added later.
> >
> > We did support some form of earlyprintk for a while (though not in the
> > same way as 32bit ARM does), until Rob introduced earlycon.
>
> Our earlyprintk up until that point was effectively a less general
> earlycon (coming up at a similar time). In fact, in v3.12 we were
> already using earlycon structures in arch/arm64/kernel/early_printk.c.
Yes, early_printk.c for arm64 is supported until Rob removed it and
replaced by earlycon at kernel v3.16
>
> Thanks,
> Mark.
Regards,
Duc Dang.
^ permalink raw reply
* [PATCH V4 09/10] trace, ras: add ARM processor error trace event
From: Baicar, Tyler @ 2016-10-24 17:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161021163410.0a827eab@gandalf.local.home>
On 10/21/2016 2:34 PM, Steven Rostedt wrote:
> On Fri, 21 Oct 2016 11:30:12 -0600
> Tyler Baicar <tbaicar@codeaurora.org> wrote:
>
>> Currently there are trace events for the various RAS
>> errors with the exception of ARM processor type errors.
>> Add a new trace event for such errors so that the user
>> will know when they occur. These trace events are
>> consistent with the ARM processor error section type
>> defined in UEFI 2.6 spec section N.2.4.4.
>>
>> Signed-off-by: Tyler Baicar <tbaicar@codeaurora.org>
> Tracing part looks fine to me.
>
> Acked-by: Steven Rostedt <rostedt@goodmis.org>
>
> -- Steve
Thank you Steve!
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH] ARM: dts: sun8i: fix the pinmux for UART1
From: Icenowy Zheng @ 2016-10-24 17:08 UTC (permalink / raw)
To: linux-arm-kernel
When the patch is applied, the allwinner,driver and allwinner,pull
properties are removed.
Although they're described to be optional in the devicetree binding,
without them, the pinmux cannot be initialized, and the uart cannot
be used.
Add them back to fix the problem, and makes the bluetooth on iNet D978
Rev2 board work.
Fixes: 82eec384249f (ARM: dts: sun8i: add pinmux for UART1 at PG)
Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
This patch is targeted on 4.9.
arch/arm/boot/dts/sun8i-a23-a33.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index 48fc24f..300a1bd 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -282,11 +282,15 @@
uart1_pins_a: uart1 at 0 {
allwinner,pins = "PG6", "PG7";
allwinner,function = "uart1";
+ allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+ allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
};
uart1_pins_cts_rts_a: uart1-cts-rts at 0 {
allwinner,pins = "PG8", "PG9";
allwinner,function = "uart1";
+ allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+ allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
};
mmc0_pins_a: mmc0 at 0 {
--
2.10.1
^ permalink raw reply related
* [RFC] ARM: memory: da8xx-ddrctl: new driver
From: Mark Rutland @ 2016-10-24 17:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477327596-16060-2-git-send-email-bgolaszewski@baylibre.com>
On Mon, Oct 24, 2016 at 06:46:36PM +0200, Bartosz Golaszewski wrote:
> Create a new driver for the da8xx DDR2/mDDR controller and implement
> support for writing to the Peripheral Bus Burst Priority Register.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> .../memory-controllers/ti-da8xx-ddrctl.txt | 20 +++
> drivers/memory/Kconfig | 8 +
> drivers/memory/Makefile | 1 +
> drivers/memory/da8xx-ddrctl.c | 187 +++++++++++++++++++++
> 4 files changed, 216 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
> create mode 100644 drivers/memory/da8xx-ddrctl.c
>
> diff --git a/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
> new file mode 100644
> index 0000000..f0eda59
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
> @@ -0,0 +1,20 @@
> +* Device tree bindings for Texas Instruments da8xx DDR2/mDDR memory controller
> +
> +The DDR2/mDDR memory controller present on Texas Instruments da8xx SoCs memory
> +maps a set of registers which allow to tweak the controller's behavior.
This is a description of the *driver*. The device itself doesn't map
some registers, it features them. Please descrive the *device*.
> +
> +Documentation:
> +OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
> +
> +Required properties:
> +
> +- compatible: "ti,da850-ddrctl" - for da850 SoC based boards
Perhaps:
"ti,da850-ddr-controller"
> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
> +{
> + const struct da8xx_ddrctl_config_knob *knob;
> + const struct da8xx_ddrctl_setting *setting;
> + u32 regprop[2], base, memsize, reg;
> + struct device_node *node, *parent;
> + void __iomem *ddrctl;
> + const char *board;
> + struct device *dev;
> + int ret;
> +
> + dev = &pdev->dev;
> + node = dev->of_node;
> +
> + /* Find the board name. */
> + for (parent = node;
> + !of_node_is_root(parent);
> + parent = of_get_parent(parent));
> +
> + ret = of_property_read_string(parent, "compatible", &board);
> + if (ret) {
> + dev_err(dev, "unable to read the soc model\n");
> + return ret;
> + }
I can see that you want to expose sysfs knobs for this, but is it really
necessary to match boards like this? It's very fragile, and commits us
to maintaining a database of board data (i.e. a board file).
I am very much not keen on that.
> +
> + /* Check if we have settings for this board. */
> + setting = da8xx_ddrctl_match_board(board);
> + if (!setting) {
> + dev_err(dev, "no settings for board '%s'\n", board);
> + return -EINVAL;
> + }
What's wrong with of_machine_is_compatible?
> +
> + /* Figure out how to map the memory for the controller. */
> + ret = of_property_read_u32_array(node, "reg", regprop, 2);
> + if (ret) {
> + dev_err(dev, "unable to parse 'reg' property\n");
> + return ret;
> + }
> +
> + base = regprop[0];
> + memsize = regprop[1];
> +
> + ddrctl = ioremap(base, memsize);
NAK. Use the proper accessors for handling reg entries.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v2 2/4] dt-bindings: Add TI SCI PM Domains
From: Kevin Hilman @ 2016-10-24 17:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7f4754bb-7ded-6ce3-9558-a45b0d2b3888@ti.com>
Dave Gerlach <d-gerlach@ti.com> writes:
> Hi,
> On 10/21/2016 01:48 PM, Kevin Hilman wrote:
>> Dave Gerlach <d-gerlach@ti.com> writes:
>>
>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>> will hook into the genpd framework and allow the TI SCI protocol to
>>> control device power states.
>>>
>>> Also, provide macros representing each device index as understood
>>> by TI SCI to be used in the device node power-domain references.
>>> These are identifiers for the K2G devices managed by the PMMC.
>>>
>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>> ---
>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 54 +++++++++++++
>>> MAINTAINERS | 2 +
>>> include/dt-bindings/genpd/k2g.h | 90 ++++++++++++++++++++++
>>> 3 files changed, 146 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>> create mode 100644 include/dt-bindings/genpd/k2g.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>> new file mode 100644
>>> index 000000000000..32f38a349656
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>> @@ -0,0 +1,54 @@
>>> +Texas Instruments TI-SCI Generic Power Domain
>>> +---------------------------------------------
>>> +
>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is
>>> +responsible for controlling the state of the IPs that are present.
>>> +Communication between the host processor running an OS and the system
>>> +controller happens through a protocol known as TI-SCI [1]. This pm domain
>>> +implementation plugs into the generic pm domain framework and makes use of
>>> +the TI SCI protocol power on and off each device when needed.
>>> +
>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>> +
>>> +PM Domain Node
>>> +==============
>>> +The PM domain node represents the global PM domain managed by the PMMC,
>>> +which in this case is the single implementation as documented by the generic
>>> +PM domain bindings in Documentation/devicetree/bindings/power/power_domain.txt.
>>> +
>>> +Required Properties:
>>> +--------------------
>>> +- compatible: should be "ti,sci-pm-domain"
>>> +- #power-domain-cells: Must be 0.
>>> +- ti,sci: Phandle to the TI SCI device to use for managing the devices.
>>>
>>> +Example:
>>> +--------------------
>>> +k2g_pds: k2g_pds {
>>
>> should use generic name like "power-contoller", e.g. k2g_pds: power-controller
>
> Ok, that makes more sense.
>
>>
>>> + compatible = "ti,sci-pm-domain";
>>> + #power-domain-cells = <0>;
>>> + ti,sci = <&pmmc>;
>>> +};
>>> +
>>> +PM Domain Consumers
>>> +===================
>>> +Hardware blocks that require SCI control over their state must provide
>>> +a reference to the sci-pm-domain they are part of and a unique device
>>> +specific ID that identifies the device.
>>> +
>>> +Required Properties:
>>> +--------------------
>>> +- power-domains: phandle pointing to the corresponding PM domain node.
>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to
>>> + be used for device control.
>>
>> This ID doesn't look right.
>>
>> Why not use #power-domain-cells = <1> and pass the index in the DT? ...
>>
>>> +See dt-bindings/genpd/k2g.h for the list of valid identifiers for k2g.
>>> +
>>> +Example:
>>> +--------------------
>>> +uart0: serial at 02530c00 {
>>> + compatible = "ns16550a";
>>> + ...
>>> + power-domains = <&k2g_pds>;
>>> + ti,sci-id = <K2G_DEV_UART0>;
>>
>> ... like this:
>>
>> power-domains = <&k2g_pds K2G_DEV_UART0>;
>
> That's how I did it in version one actually. I was able to define my
> own xlate function to parse the phandle and get that index, but Ulf
> pointed me to this series by Jon Hunter [1] that simplified genpd
> providers and dropped the concept of adding your own xlate. This locks
> the onecell approach to using a fixed static array of genpds that get
> indexed into (without passing the index to the provider, just the
> genpd that's looked up), which doesn't fit our usecase, as we don't
> want a 1 to 1 genpd to device mapping based on the comments provided
> in v1. Now we just use the genpd device attach/detach hooks to parse
> the sci-id and then use it in the genpd device start/stop hooks.
Ah, right. I remember now. This approach allows you to use a single
genpd as discussed earlier.
Makes sense now, suggestion retracted.
Kevin
^ permalink raw reply
* Disabling an interrupt in the handler locks the system up
From: Thomas Gleixner @ 2016-10-24 16:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <580E3308.4050507@free.fr>
On Mon, 24 Oct 2016, Mason wrote:
>
> For the record, setting the IRQ_DISABLE_UNLAZY flag for this device
> makes the system lock-up disappear.
The way how lazy irq disabling works is:
1) Interrupt is marked disabled in software, but the hardware is not masked
2) If the interrupt fires befor the interrupt is reenabled, then it's
masked at the hardware level in the low level interrupt flow handler.
I have no idea why that does not work on your hardware. You might
instrument handle_level_irq() to see what effect that has.
Thanks,
tglx
^ permalink raw reply
* [GIT PULL 4/4] Broadcom defconfig-arm64 changes for 4.9 (part 2)
From: Florian Fainelli @ 2016-10-24 16:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAOesGMjJ3_rPUke8B5yW_6Q4NyP1sb-wqY5jfcs1_+UhGQK9_g@mail.gmail.com>
On 10/23/2016 04:50 PM, Olof Johansson wrote:
> On Sun, Oct 23, 2016 at 2:55 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 09/30/2016 12:23 PM, Florian Fainelli wrote:
>>> The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc:
>>>
>>> Linux 4.8-rc1 (2016-08-07 18:18:00 -0700)
>>>
>>> are available in the git repository at:
>>>
>>> http://github.com/Broadcom/stblinux.git tags/arm-soc/for-4.9/defconfig-arm64
>>>
>>> for you to fetch changes up to 51e3fb1d3f514cd336faf185df73b25fca194773:
>>>
>>> Merge tag 'bcm2835-defconfig-64-next-2016-09-22' into defconfig-arm64/next (2016-09-30 12:02:29 -0700)
>>>
>>> ----------------------------------------------------------------
>>> This pull request contains Broadcom ARM64-based SoCs defconfig changes for 4.9,
>>> please pull the following changes:
>>>
>>> - Eric updates the ARMv8 defconfig to contain everything that is needed to run
>>> a 64-bit kernel on the Raspberry Pi 3
>>>
>>> ----------------------------------------------------------------
>>> Eric Anholt (1):
>>> arm64: Add BCM2835 (Raspberry Pi 3) support to the defconfig
>>>
>>> Florian Fainelli (1):
>>> Merge tag 'bcm2835-defconfig-64-next-2016-09-22' into defconfig-arm64/next
>>>
>>> arch/arm64/configs/defconfig | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>
>> Arnd, Kevin, Olof,
>>
>> Following Olof's response here:
>>
>> https://www.spinics.net/lists/arm-kernel/msg534687.html
>>
>> do you think we could merge these for 4.9-rcX? Let me know if I should
>> send a fresh pull request for that.
>
> Main reason to respin would be if you need to rebase due to other
> changes that have gone in, or if you expect to have other material
> that should be based on a 4.9-rc base.
No dependencies, this should apply cleanly to 4.9-rc. Thanks!
--
Florian
^ permalink raw reply
* [PATCH v3 3/8] PM / Domains: Allow domain power states to be read from DT
From: Lina Iyer @ 2016-10-24 16:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6ed96121-5040-474d-2d71-7927e8567c50@arm.com>
Hi Sudeep,
On Mon, Oct 24 2016 at 07:39 -0600, Sudeep Holla wrote:
>
>
>On 14/10/16 18:47, Lina Iyer wrote:
>>This patch allows domains to define idle states in the DT. SoC's can
>>define domain idle states in DT using the "domain-idle-states" property
>>of the domain provider. Add API to read the idle states from DT that can
>>be set in the genpd object.
>>
>>This patch is based on the original patch by Marc Titinger.
>>
>>Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>---
>> drivers/base/power/domain.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/pm_domain.h | 8 ++++
>> 2 files changed, 102 insertions(+)
>>
>>diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>index 37ab7f1..9af75ba 100644
>>--- a/drivers/base/power/domain.c
>>+++ b/drivers/base/power/domain.c
>>@@ -1916,6 +1916,100 @@ out:
>> return ret ? -EPROBE_DEFER : 0;
>> }
>> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>>+
>>+static const struct of_device_id idle_state_match[] = {
>>+ { .compatible = "arm,idle-state", },
>>+ { }
>>+};
>>+
>
>I still think it's better to have another compatible to serve this
>purpose. We don't want to end up creating genpd domains just because
>they are "arm,idle-state" compatible IMO ?
>
>I agree you can prevent it checking for OSC mode support in the
>firmware. But I want to understand if you have any strong reasons for
>avoiding that approach.
>
Why are you still held up with OSI/PC PSCI modes? I repeat again this
series is not about any of that, it is just about PM domains. PM domains
have idle states and the idle-state description is similar in definition
to arm,idle-state and therefore uses the same compatible. There is no
point re-defining something that already exists in the kernel.
I was able to find the original thread, where we discussed this [1].
I suggest, you read about PM domains and its idle states and understand
this series in the context of PM domains.
Thanks,
Lina
[1]. http://www.serverphorums.com/read.php?12,1303996
^ permalink raw reply
* [RFC] ARM: memory: da8xx-ddrctl: new driver
From: Bartosz Golaszewski @ 2016-10-24 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477327596-16060-1-git-send-email-bgolaszewski@baylibre.com>
Create a new driver for the da8xx DDR2/mDDR controller and implement
support for writing to the Peripheral Bus Burst Priority Register.
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
.../memory-controllers/ti-da8xx-ddrctl.txt | 20 +++
drivers/memory/Kconfig | 8 +
drivers/memory/Makefile | 1 +
drivers/memory/da8xx-ddrctl.c | 187 +++++++++++++++++++++
4 files changed, 216 insertions(+)
create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
create mode 100644 drivers/memory/da8xx-ddrctl.c
diff --git a/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
new file mode 100644
index 0000000..f0eda59
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
@@ -0,0 +1,20 @@
+* Device tree bindings for Texas Instruments da8xx DDR2/mDDR memory controller
+
+The DDR2/mDDR memory controller present on Texas Instruments da8xx SoCs memory
+maps a set of registers which allow to tweak the controller's behavior.
+
+Documentation:
+OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
+
+Required properties:
+
+- compatible: "ti,da850-ddrctl" - for da850 SoC based boards
+- reg: a tuple containing the base address of the memory
+ controller and the size of the memory area to map
+
+Example for da850 shown below.
+
+ddrctl {
+ compatible = "ti,da850-ddrctl";
+ reg = <0xB0000000 0x100>;
+};
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 4b4c0c3..ec80e35 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -134,6 +134,14 @@ config MTK_SMI
mainly help enable/disable iommu and control the power domain and
clocks for each local arbiter.
+config DA8XX_DDRCTL
+ bool "Texas Instruments da8xx DDR2/mDDR driver"
+ depends on ARCH_DAVINCI_DA8XX
+ help
+ This driver is for the DDR2/mDDR Memory Controller present on
+ Texas Instruments da8xx SoCs. It's used to tweak various memory
+ controller configuration options.
+
source "drivers/memory/samsung/Kconfig"
source "drivers/memory/tegra/Kconfig"
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index b20ae38..e88097fb 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o
obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o
obj-$(CONFIG_JZ4780_NEMC) += jz4780-nemc.o
obj-$(CONFIG_MTK_SMI) += mtk-smi.o
+obj-$(CONFIG_DA8XX_DDRCTL) += da8xx-ddrctl.o
obj-$(CONFIG_SAMSUNG_MC) += samsung/
obj-$(CONFIG_TEGRA_MC) += tegra/
diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
new file mode 100644
index 0000000..756a6f3
--- /dev/null
+++ b/drivers/memory/da8xx-ddrctl.c
@@ -0,0 +1,187 @@
+/*
+ * TI da8xx DDR2/mDDR controller driver
+ *
+ * Copyright (C) 2016 BayLibre SAS
+ *
+ * Author:
+ * Bartosz Golaszewski <bgolaszewski@baylibre.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.
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+
+struct da8xx_ddrctl_config_knob {
+ const char *name;
+ u32 reg;
+ u32 mask;
+ u32 offset;
+};
+
+static const struct da8xx_ddrctl_config_knob da8xx_ddrctl_knobs[] = {
+ {
+ .name = "da850-pbbpr",
+ .reg = 0x20,
+ .mask = 0xffffff00,
+ .offset = 0,
+ },
+};
+
+struct da8xx_ddrctl_setting {
+ const char *name;
+ u32 val;
+};
+
+struct da8xx_ddrctl_board_settings {
+ const char *board;
+ const struct da8xx_ddrctl_setting *settings;
+};
+
+static const struct da8xx_ddrctl_setting da850_lcdk_ddrctl_settings[] = {
+ {
+ .name = "da850-pbbpr",
+ .val = 0x20,
+ },
+ { }
+};
+
+static const struct da8xx_ddrctl_board_settings da8xx_ddrctl_board_confs[] = {
+ {
+ .board = "ti,da850-lcdk",
+ .settings = da850_lcdk_ddrctl_settings,
+ },
+};
+
+static const struct da8xx_ddrctl_config_knob *
+da8xx_ddrctl_match_knob(const struct da8xx_ddrctl_setting *setting)
+{
+ const struct da8xx_ddrctl_config_knob *knob;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(da8xx_ddrctl_knobs); i++) {
+ knob = &da8xx_ddrctl_knobs[i];
+
+ if (strcmp(knob->name, setting->name) == 0) {
+ return knob;
+ }
+ }
+
+ return NULL;
+}
+
+static const struct da8xx_ddrctl_setting *
+da8xx_ddrctl_match_board(const char *board)
+{
+ const struct da8xx_ddrctl_board_settings *board_settings;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(da8xx_ddrctl_board_confs); i++) {
+ board_settings = &da8xx_ddrctl_board_confs[0];
+
+ if (strcmp(board, board_settings->board) == 0)
+ return board_settings->settings;
+ }
+
+ return NULL;
+}
+
+static int da8xx_ddrctl_probe(struct platform_device *pdev)
+{
+ const struct da8xx_ddrctl_config_knob *knob;
+ const struct da8xx_ddrctl_setting *setting;
+ u32 regprop[2], base, memsize, reg;
+ struct device_node *node, *parent;
+ void __iomem *ddrctl;
+ const char *board;
+ struct device *dev;
+ int ret;
+
+ dev = &pdev->dev;
+ node = dev->of_node;
+
+ /* Find the board name. */
+ for (parent = node;
+ !of_node_is_root(parent);
+ parent = of_get_parent(parent));
+
+ ret = of_property_read_string(parent, "compatible", &board);
+ if (ret) {
+ dev_err(dev, "unable to read the soc model\n");
+ return ret;
+ }
+
+ /* Check if we have settings for this board. */
+ setting = da8xx_ddrctl_match_board(board);
+ if (!setting) {
+ dev_err(dev, "no settings for board '%s'\n", board);
+ return -EINVAL;
+ }
+
+ /* Figure out how to map the memory for the controller. */
+ ret = of_property_read_u32_array(node, "reg", regprop, 2);
+ if (ret) {
+ dev_err(dev, "unable to parse 'reg' property\n");
+ return ret;
+ }
+
+ base = regprop[0];
+ memsize = regprop[1];
+
+ ddrctl = ioremap(base, memsize);
+ if (!ddrctl) {
+ dev_err(dev, "unable to map memory controller registers\n");
+ return -EIO;
+ }
+
+ for (; setting->name; setting++) {
+ knob = da8xx_ddrctl_match_knob(setting);
+ if (!knob) {
+ dev_warn(dev,
+ "no such config option: %s\n", setting->name);
+ continue;
+ }
+
+ if (knob->reg > (memsize - sizeof(u32))) {
+ dev_warn(dev,
+ "register offset of '%s' exceeds mapped memory size\n",
+ knob->name);
+ continue;
+ }
+
+ reg = __raw_readl(ddrctl + knob->reg);
+ reg &= knob->mask;
+ reg |= setting->val << knob->offset;
+
+ dev_dbg(dev, "writing 0x%08x to %s\n", reg, setting->name);
+
+ __raw_writel(reg, ddrctl + knob->reg);
+ }
+
+ iounmap(ddrctl);
+
+ return 0;
+}
+
+static const struct of_device_id da8xx_ddrctl_of_match[] = {
+ { .compatible = "ti,da850-ddrctl", },
+ { },
+};
+
+static struct platform_driver da8xx_ddrctl_driver = {
+ .probe = da8xx_ddrctl_probe,
+ .driver = {
+ .name = "da8xx-ddrctl",
+ .of_match_table = da8xx_ddrctl_of_match,
+ },
+};
+module_platform_driver(da8xx_ddrctl_driver);
+
+MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
+MODULE_DESCRIPTION("TI da8xx DDR2/mDDR controller driver");
+MODULE_LICENSE("GPL v2");
--
2.9.3
^ permalink raw reply related
* [RFC] da850: DDR2/mDDR memory controller driver
From: Bartosz Golaszewski @ 2016-10-24 16:46 UTC (permalink / raw)
To: linux-arm-kernel
This is a follow-up for the series[1] adding new bus and memory drivers
to better support the TI LCD controller on the da850-lcdk board.
The general consensus of the discussion that followed was that DT is
not the right tool for this kind of SoC performance tweaks.
In order to avoid committing to stable DT bindings, we only introduce
two common properties (compatible and reg) while the configuration
register values are hard-coded for each board (currently only lcdk).
I'm sending a single patch this time as RFC to get some reviews and
see it the approach is viewed as correct.
[1] https://lkml.org/lkml/2016/10/17/613
Bartosz Golaszewski (1):
ARM: memory: da8xx-ddrctl: new driver
.../memory-controllers/ti-da8xx-ddrctl.txt | 20 +++
drivers/memory/Kconfig | 8 +
drivers/memory/Makefile | 1 +
drivers/memory/da8xx-ddrctl.c | 187 +++++++++++++++++++++
4 files changed, 216 insertions(+)
create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
create mode 100644 drivers/memory/da8xx-ddrctl.c
--
2.9.3
^ permalink raw reply
* [PATCH/RFT v2 17/17] ARM: dts: da850: add usb device node
From: ahaslam at baylibre.com @ 2016-10-24 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-1-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
This adds the usb (ohci) device node for the da850 soc.
Also it enables it for the lcdk board
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/boot/dts/da850-lcdk.dts | 8 ++++++++
arch/arm/boot/dts/da850.dtsi | 8 ++++++++
2 files changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 7b8ab21..fa91339 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -86,6 +86,14 @@
};
};
+&usb_phy {
+ status = "okay";
+};
+
+&usb {
+ status = "okay";
+};
+
&serial2 {
pinctrl-names = "default";
pinctrl-0 = <&serial2_rxtx_pins>;
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 33fcdce..ec2cec3 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -381,6 +381,14 @@
#phy-cells = <1>;
status = "disabled";
};
+ usb: usb at 0225000 {
+ compatible = "ti,da830-ohci";
+ reg = <0x225000 0x1000>;
+ interrupts = <59>;
+ phys = <&usb_phy 1>;
+ phy-names = "usb-phy";
+ status = "disabled";
+ };
gpio: gpio at 226000 {
compatible = "ti,dm6441-gpio";
gpio-controller;
--
1.9.1
^ permalink raw reply related
* [PATCH/RFT v2 16/17] USB: ohci-da8xx: Allow probing from DT
From: ahaslam at baylibre.com @ 2016-10-24 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-1-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
This adds the compatible string to the ohci driver
to be able to probe from DT
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/usb/host/ohci-da8xx.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index bebc3f0..1a8db25 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -273,6 +273,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
}
/*-------------------------------------------------------------------------*/
+#ifdef CONFIG_OF
+static const struct of_device_id da8xx_ohci_ids[] = {
+ { .compatible = "ti,da830-ohci" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, da8xx_ohci_ids);
+#endif
static int ohci_da8xx_probe(struct platform_device *pdev)
{
@@ -421,6 +428,7 @@ static int ohci_da8xx_resume(struct platform_device *dev)
#endif
.driver = {
.name = "ohci",
+ .of_match_table = da8xx_ohci_ids,
},
};
--
1.9.1
^ permalink raw reply related
* [PATCH/RFT v2 15/17] usb: host: ohci-da8xx: Add devicetree bindings documentation
From: ahaslam at baylibre.com @ 2016-10-24 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-1-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
This patch documents the device tree bindings required for
the ohci controller found in TI da8xx family of SoC's
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
.../devicetree/bindings/usb/ohci-da8xx.txt | 39 ++++++++++++++++++++++
1 file changed, 39 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ohci-da8xx.txt
diff --git a/Documentation/devicetree/bindings/usb/ohci-da8xx.txt b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
new file mode 100644
index 0000000..4251c84
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ohci-da8xx.txt
@@ -0,0 +1,39 @@
+DA8XX USB OHCI controller
+
+Required properties:
+
+ - compatible: Should be "ti,da830-ohci"
+ - reg: Should contain one register range i.e. start and length
+ - interrupts: Description of the interrupt line
+ - phys: Phandle for the PHY device
+ - phy-names: Should be "usb-phy"
+
+Optional properties:
+ - vbus-supply: Regulator that controls vbus power
+
+Example for omap138-lck:
+
+vbus_fixed: fixed-regulator-vbus {
+ compatible = "regulator-fixed";
+ gpio = <&gpio 109 0>;
+ oc-gpio = <&gpio 36 0>;
+ regulator-boot-on;
+ enable-active-high;
+ regulator-name = "vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+};
+
+usb_phy: usb-phy {
+ compatible = "ti,da830-usb-phy";
+ #phy-cells = <1>;
+ status = "disabled";
+};
+usb: usb at 0225000 {
+ compatible = "ti,da830-ohci";
+ reg = <0x225000 0x1000>;
+ interrupts = <59>;
+ phys = <&usb_phy 1>;
+ phy-names = "usb-phy";
+ status = "disabled";
+};
--
1.9.1
^ permalink raw reply related
* [PATCH/RFT v2 14/17] ARM: davinci: register the usb20_phy clock on the DT file
From: ahaslam at baylibre.com @ 2016-10-24 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-1-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
The usb20_phy clock needs to be registered for the driver to be able
to get and enable a clock. Currently the usb phy clocks are registered
form board files, which will not be called during a device tree based
boot.
To be able to probe correctly usb form a device tree boot, register
the usb phy clocks form the DT specific init.
Unfourtunatly, davinci does not have proper clock support on device tree
yet, so by registering the clock form de DT specific file we are
forced to hardcode the parent clock, and cannot select refclkin as
parent for any of the phy clocks of the da850 family.
As none of the current da850 based boards currently in mainline use
refclkin as source. I guess we can live with this limitation until clocks
are correctly represented through CCF/device tree.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/da8xx-dt.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index c9f7e92..7947267 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -45,6 +45,8 @@
static void __init da850_init_machine(void)
{
+ da8xx_register_usb20_phy_clk(false);
+ da8xx_register_usb11_phy_clk(false);
of_platform_default_populate(NULL, da850_auxdata_lookup, NULL);
}
--
1.9.1
^ permalink raw reply related
* [PATCH/RFT v2 13/17] USB: da8xx: use ohci priv data instead of globals
From: ahaslam at baylibre.com @ 2016-10-24 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-1-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
Instead of global variables, use the extra_priv_size of the ohci driver
to add a reference to driver private data.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
drivers/usb/host/ohci-da8xx.c | 135 ++++++++++++++++++++++++------------------
1 file changed, 79 insertions(+), 56 deletions(-)
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index f4bda4d..bebc3f0 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -37,60 +37,66 @@ static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq,
u16 wValue, u16 wIndex, char *buf, u16 wLength);
static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
-static struct clk *usb11_clk;
-static struct phy *usb11_phy;
-static struct regulator *vbus_reg;
-struct notifier_block nb;
-
-/* Over-current indicator change flag */
-static int ocic_flag;
+struct da8xx_ohci_hcd {
+ struct usb_hcd *hcd;
+ struct clk *usb11_clk;
+ struct phy *usb11_phy;
+ struct regulator *vbus_reg;
+ struct notifier_block nb;
+ int ocic_flag;
+};
+#define to_da8xx_ohci(hcd) (struct da8xx_ohci_hcd *)(hcd_to_ohci(hcd)->priv)
-static int ohci_da8xx_enable(void)
+static int ohci_da8xx_enable(struct usb_hcd *hcd)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
int ret;
- ret = clk_prepare_enable(usb11_clk);
+ ret = clk_prepare_enable(da8xx_ohci->usb11_clk);
if (ret)
return ret;
- ret = phy_init(usb11_phy);
+ ret = phy_init(da8xx_ohci->usb11_phy);
if (ret)
goto err_phy_init;
- ret = phy_power_on(usb11_phy);
+ ret = phy_power_on(da8xx_ohci->usb11_phy);
if (ret)
goto err_phy_power_on;
return 0;
err_phy_power_on:
- phy_exit(usb11_phy);
+ phy_exit(da8xx_ohci->usb11_phy);
err_phy_init:
- clk_disable_unprepare(usb11_clk);
+ clk_disable_unprepare(da8xx_ohci->usb11_clk);
return ret;
}
-static void ohci_da8xx_disable(void)
+static void ohci_da8xx_disable(struct usb_hcd *hcd)
{
- phy_power_off(usb11_phy);
- phy_exit(usb11_phy);
- clk_disable_unprepare(usb11_clk);
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
+
+ phy_power_off(da8xx_ohci->usb11_phy);
+ phy_exit(da8xx_ohci->usb11_phy);
+ clk_disable_unprepare(da8xx_ohci->usb11_clk);
}
-static int ohci_da8xx_set_power(int on)
+static int ohci_da8xx_set_power(struct usb_hcd *hcd, int on)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
int ret = 0;
- if (!vbus_reg)
+ if (!da8xx_ohci->vbus_reg)
return 0;
if (on) {
- ret = regulator_enable(vbus_reg);
+ ret = regulator_enable(da8xx_ohci->vbus_reg);
if (ret)
pr_err("fail to enable regulator: %d\n", ret);
} else {
- ret = regulator_disable(vbus_reg);
+ ret = regulator_disable(da8xx_ohci->vbus_reg);
if (ret)
pr_err("fail to disable regulator: %d\n", ret);
}
@@ -98,17 +104,22 @@ static int ohci_da8xx_set_power(int on)
return ret;
}
-static int ohci_da8xx_get_power(void)
+static int ohci_da8xx_get_power(struct usb_hcd *hcd)
{
- if (!vbus_reg)
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
+
+ if (!da8xx_ohci->vbus_reg)
return 1;
- return regulator_is_enabled(vbus_reg);
+ return regulator_is_enabled(da8xx_ohci->vbus_reg);
}
-static int ohci_da8xx_get_oci(void)
+static int ohci_da8xx_get_oci(struct usb_hcd *hcd)
{
- if (regulator_get_mode(vbus_reg) == REGULATOR_MODE_OVERCURRENT)
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
+
+ if (regulator_get_mode(da8xx_ohci->vbus_reg) ==
+ REGULATOR_MODE_OVERCURRENT)
return 1;
return 0;
@@ -117,10 +128,13 @@ static int ohci_da8xx_get_oci(void)
static int ohci_da8xx_regulator_event(struct notifier_block *nb,
unsigned long event, void *data)
{
+ struct da8xx_ohci_hcd *da8xx_ohci =
+ container_of(nb, struct da8xx_ohci_hcd, nb);
+
if (event & REGULATOR_EVENT_OVER_CURRENT) {
- ocic_flag = 1;
- if (ohci_da8xx_get_oci())
- ohci_da8xx_set_power(0);
+ da8xx_ohci->ocic_flag = 1;
+ if (ohci_da8xx_get_oci(da8xx_ohci->hcd))
+ ohci_da8xx_set_power(da8xx_ohci->hcd, 0);
}
return 0;
@@ -130,12 +144,13 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
{
struct device *dev = hcd->self.controller;
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
int result;
u32 rh_a;
dev_dbg(dev, "starting USB controller\n");
- result = ohci_da8xx_enable();
+ result = ohci_da8xx_enable(hcd);
if (result < 0)
return result;
@@ -147,7 +162,7 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
result = ohci_setup(hcd);
if (result < 0) {
- ohci_da8xx_disable();
+ ohci_da8xx_disable(hcd);
return result;
}
@@ -159,7 +174,7 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
*/
rh_a = ohci_readl(ohci, &ohci->regs->roothub.a);
- if (vbus_reg) {
+ if (da8xx_ohci->vbus_reg) {
rh_a &= ~RH_A_NPS;
rh_a |= RH_A_PSM;
rh_a &= ~RH_A_NOCP;
@@ -176,10 +191,11 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
*/
static int ohci_da8xx_hub_status_data(struct usb_hcd *hcd, char *buf)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
int length = orig_ohci_hub_status_data(hcd, buf);
/* See if we have OCIC flag set */
- if (ocic_flag) {
+ if (da8xx_ohci->ocic_flag) {
dev_dbg(hcd->self.controller, "over-current indicator change "
"on port 1\n");
@@ -197,6 +213,7 @@ static int ohci_da8xx_hub_status_data(struct usb_hcd *hcd, char *buf)
static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
u16 wIndex, char *buf, u16 wLength)
{
+ struct da8xx_ohci_hcd *da8xx_ohci = to_da8xx_ohci(hcd);
struct device *dev = hcd->self.controller;
int temp;
@@ -211,15 +228,15 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp = roothub_portstatus(hcd_to_ohci(hcd), wIndex - 1);
/* The port power status (PPS) bit defaults to 1 */
- if (ohci_da8xx_get_power() == 0)
+ if (ohci_da8xx_get_power(hcd) == 0)
temp &= ~RH_PS_PPS;
/* The port over-current indicator (POCI) bit is always 0 */
- if (ohci_da8xx_get_oci() > 0)
+ if (ohci_da8xx_get_oci(hcd) > 0)
temp |= RH_PS_POCI;
/* The over-current indicator change (OCIC) bit is 0 too */
- if (ocic_flag)
+ if (da8xx_ohci->ocic_flag)
temp |= RH_PS_OCIC;
put_unaligned(cpu_to_le32(temp), (__le32 *)buf);
@@ -240,13 +257,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex, "POWER");
- return ohci_da8xx_set_power(temp) ? -EPIPE : 0;
+ return ohci_da8xx_set_power(hcd, temp) ? -EPIPE : 0;
case USB_PORT_FEAT_C_OVER_CURRENT:
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex,
"C_OVER_CURRENT");
- ocic_flag = temp;
+ da8xx_ohci->ocic_flag = temp;
return 0;
}
}
@@ -259,6 +276,7 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
static int ohci_da8xx_probe(struct platform_device *pdev)
{
+ struct da8xx_ohci_hcd *da8xx_ohci;
struct usb_hcd *hcd;
struct resource *mem;
int error, irq;
@@ -268,30 +286,34 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
if (!hcd)
return -ENOMEM;
- usb11_clk = devm_clk_get(&pdev->dev, "usb11");
- if (IS_ERR(usb11_clk)) {
- if (PTR_ERR(usb11_clk) != -EPROBE_DEFER)
+ da8xx_ohci = to_da8xx_ohci(hcd);
+ da8xx_ohci->hcd = hcd;
+
+ da8xx_ohci->usb11_clk = devm_clk_get(&pdev->dev, "usb11");
+ if (IS_ERR(da8xx_ohci->usb11_clk)) {
+ if (PTR_ERR(da8xx_ohci->usb11_clk) != -EPROBE_DEFER)
dev_err(&pdev->dev, "Failed to get clock.\n");
- return PTR_ERR(usb11_clk);
+ return PTR_ERR(da8xx_ohci->usb11_clk);
}
- usb11_phy = devm_phy_get(&pdev->dev, "usb-phy");
- if (IS_ERR(usb11_phy)) {
- if (PTR_ERR(usb11_phy) != -EPROBE_DEFER)
+ da8xx_ohci->usb11_phy = devm_phy_get(&pdev->dev, "usb-phy");
+ if (IS_ERR(da8xx_ohci->usb11_phy)) {
+ if (PTR_ERR(da8xx_ohci->usb11_phy) != -EPROBE_DEFER)
dev_err(&pdev->dev, "Failed to get phy.\n");
- return PTR_ERR(usb11_phy);
+ return PTR_ERR(da8xx_ohci->usb11_phy);
}
- vbus_reg = devm_regulator_get(&pdev->dev, "vbus");
- if (IS_ERR(vbus_reg)) {
- if (PTR_ERR(vbus_reg) != -EPROBE_DEFER)
+ da8xx_ohci->vbus_reg = devm_regulator_get(&pdev->dev, "vbus");
+ if (IS_ERR(da8xx_ohci->vbus_reg)) {
+ if (PTR_ERR(da8xx_ohci->vbus_reg) != -EPROBE_DEFER)
dev_err(&pdev->dev, "Failed to get regulator.\n");
- return PTR_ERR(vbus_reg);
+ return PTR_ERR(da8xx_ohci->vbus_reg);
}
- if (vbus_reg) {
- nb.notifier_call = ohci_da8xx_regulator_event;
- error = devm_regulator_register_notifier(vbus_reg, &nb);
+ if (da8xx_ohci->vbus_reg) {
+ da8xx_ohci->nb.notifier_call = ohci_da8xx_regulator_event;
+ error = devm_regulator_register_notifier(da8xx_ohci->vbus_reg,
+ &da8xx_ohci->nb);
if (error) {
dev_err(&pdev->dev,
"Could not register regulator notifier\n");
@@ -354,7 +376,7 @@ static int ohci_da8xx_suspend(struct platform_device *pdev,
if (ret)
return ret;
- ohci_da8xx_disable();
+ ohci_da8xx_disable(hcd);
hcd->state = HC_STATE_SUSPENDED;
return ret;
@@ -370,7 +392,7 @@ static int ohci_da8xx_resume(struct platform_device *dev)
msleep(5);
ohci->next_statechange = jiffies;
- ret = ohci_da8xx_enable();
+ ret = ohci_da8xx_enable(hcd);
if (ret)
return ret;
@@ -382,7 +404,8 @@ static int ohci_da8xx_resume(struct platform_device *dev)
#endif
static const struct ohci_driver_overrides da8xx_overrides __initconst = {
- .reset = ohci_da8xx_reset
+ .reset = ohci_da8xx_reset,
+ .extra_priv_size = sizeof(struct da8xx_ohci_hcd),
};
/*
--
1.9.1
^ permalink raw reply related
* [PATCH/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent
From: ahaslam at baylibre.com @ 2016-10-24 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-1-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
Currently, the da8xx ohci driver uses a set of gpios and callbacks in
board files to handle vbus and overcurrent irqs form the power supply.
However, this does not play nice when moving to a DT based boot were
we wont have board files.
Instead of requesting and handling the gpio, use the regulator framework
to take care of enabling and disabling vbus power. This has the benefit
that we dont need to pass any more platform data to the driver:
These will be handled by the regulator framework:
set_power -> regulator_enable/regulator_disable
get_power -> regulator_is_enabled
get_oci -> regulator_get_mode
ocic_notify -> regulator notification
We can keep the default potpgt and use the regulator start delay instead:
potpgt -> regulator startup delay time
The hawk board does not have a GPIO/OVERCURRENT gpio to control vbus,
(they should not have been decleared/reserved) so, just remove those
definitions from the hwk board file.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/board-da830-evm.c | 97 ++++++++----------------
arch/arm/mach-davinci/board-omapl138-hawk.c | 96 +-----------------------
arch/arm/mach-davinci/include/mach/da8xx.h | 2 +-
arch/arm/mach-davinci/usb-da8xx.c | 3 +-
drivers/usb/host/ohci-da8xx.c | 111 ++++++++++++++++++----------
include/linux/platform_data/usb-davinci.h | 19 -----
6 files changed, 105 insertions(+), 223 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index d12fcf5..d6f9f8a 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -27,6 +27,7 @@
#include <linux/platform_data/mtd-davinci-aemif.h>
#include <linux/platform_data/spi-davinci.h>
#include <linux/regulator/machine.h>
+#include <linux/regulator/fixed.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
@@ -48,61 +49,35 @@
-1
};
-static da8xx_ocic_handler_t da830_evm_usb_ocic_handler;
+static struct regulator_consumer_supply usb_ohci_consumer_supply =
+ REGULATOR_SUPPLY("vbus", "ohci");
-static int da830_evm_usb_set_power(int on)
-{
- gpio_set_value(ON_BD_USB_DRV, on);
- return 0;
-}
-
-static int da830_evm_usb_get_power(void)
-{
- return gpio_get_value(ON_BD_USB_DRV);
-}
-
-static int da830_evm_usb_get_oci(void)
-{
- return !gpio_get_value(ON_BD_USB_OVC);
-}
-
-static irqreturn_t da830_evm_usb_ocic_irq(int, void *);
-
-static int da830_evm_usb_ocic_notify(da8xx_ocic_handler_t handler)
-{
- int irq = gpio_to_irq(ON_BD_USB_OVC);
- int error = 0;
-
- if (handler != NULL) {
- da830_evm_usb_ocic_handler = handler;
-
- error = request_irq(irq, da830_evm_usb_ocic_irq,
- IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
- "OHCI over-current indicator", NULL);
- if (error)
- pr_err("%s: could not request IRQ to watch over-current indicator changes\n",
- __func__);
- } else
- free_irq(irq, NULL);
-
- return error;
-}
-
-static struct da8xx_ohci_root_hub da830_evm_usb11_pdata = {
- .set_power = da830_evm_usb_set_power,
- .get_power = da830_evm_usb_get_power,
- .get_oci = da830_evm_usb_get_oci,
- .ocic_notify = da830_evm_usb_ocic_notify,
+static struct regulator_init_data usb_ohci_initdata = {
+ .consumer_supplies = &usb_ohci_consumer_supply,
+ .num_consumer_supplies = 1,
+ .constraints = {
+ .valid_ops_mask = REGULATOR_CHANGE_STATUS,
+ },
+};
- /* TPS2065 switch @ 5V */
- .potpgt = (3 + 1) / 2, /* 3 ms max */
+static struct fixed_voltage_config usb_ohci_config = {
+ .supply_name = "vbus",
+ .microvolts = 5000000,
+ .gpio = ON_BD_USB_DRV,
+ .oc_gpio = ON_BD_USB_OVC,
+ .has_oc_gpio = 1,
+ .enable_high = 1,
+ .enabled_at_boot = 0,
+ .init_data = &usb_ohci_initdata,
};
-static irqreturn_t da830_evm_usb_ocic_irq(int irq, void *dev_id)
-{
- da830_evm_usb_ocic_handler(&da830_evm_usb11_pdata);
- return IRQ_HANDLED;
-}
+static struct platform_device da8xx_usb11_regulator = {
+ .name = "reg-fixed-voltage",
+ .id = 0,
+ .dev = {
+ .platform_data = &usb_ohci_config,
+ },
+};
static __init void da830_evm_usb_init(void)
{
@@ -144,23 +119,11 @@ static __init void da830_evm_usb_init(void)
return;
}
- ret = gpio_request(ON_BD_USB_DRV, "ON_BD_USB_DRV");
- if (ret) {
- pr_err("%s: failed to request GPIO for USB 1.1 port power control: %d\n",
- __func__, ret);
- return;
- }
- gpio_direction_output(ON_BD_USB_DRV, 0);
-
- ret = gpio_request(ON_BD_USB_OVC, "ON_BD_USB_OVC");
- if (ret) {
- pr_err("%s: failed to request GPIO for USB 1.1 port over-current indicator: %d\n",
- __func__, ret);
- return;
- }
- gpio_direction_input(ON_BD_USB_OVC);
+ ret = platform_device_register(&da8xx_usb11_regulator);
+ if (ret)
+ pr_warn("fail to add ohci regulator\n");
- ret = da8xx_register_usb11(&da830_evm_usb11_pdata);
+ ret = da8xx_register_usb11();
if (ret)
pr_warn("%s: USB 1.1 registration failed: %d\n", __func__, ret);
}
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index 1d31f45..2df34cb 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -28,9 +28,6 @@
#define DA850_HAWK_MMCSD_CD_PIN GPIO_TO_PIN(3, 12)
#define DA850_HAWK_MMCSD_WP_PIN GPIO_TO_PIN(3, 13)
-#define DA850_USB1_VBUS_PIN GPIO_TO_PIN(2, 4)
-#define DA850_USB1_OC_PIN GPIO_TO_PIN(6, 13)
-
static short omapl138_hawk_mii_pins[] __initdata = {
DA850_MII_TXEN, DA850_MII_TXCLK, DA850_MII_COL, DA850_MII_TXD_3,
DA850_MII_TXD_2, DA850_MII_TXD_1, DA850_MII_TXD_0, DA850_MII_RXER,
@@ -181,76 +178,10 @@ static __init void omapl138_hawk_mmc_init(void)
gpio_free(DA850_HAWK_MMCSD_CD_PIN);
}
-static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id);
-static da8xx_ocic_handler_t hawk_usb_ocic_handler;
-
-static const short da850_hawk_usb11_pins[] = {
- DA850_GPIO2_4, DA850_GPIO6_13,
- -1
-};
-
-static int hawk_usb_set_power(int on)
-{
- gpio_set_value(DA850_USB1_VBUS_PIN, on);
- return 0;
-}
-
-static int hawk_usb_get_power(void)
-{
- return gpio_get_value(DA850_USB1_VBUS_PIN);
-}
-
-static int hawk_usb_get_oci(void)
-{
- return !gpio_get_value(DA850_USB1_OC_PIN);
-}
-
-static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
-{
- int irq = gpio_to_irq(DA850_USB1_OC_PIN);
- int error = 0;
-
- if (handler != NULL) {
- hawk_usb_ocic_handler = handler;
-
- error = request_irq(irq, omapl138_hawk_usb_ocic_irq,
- IRQF_TRIGGER_RISING |
- IRQF_TRIGGER_FALLING,
- "OHCI over-current indicator", NULL);
- if (error)
- pr_err("%s: could not request IRQ to watch "
- "over-current indicator changes\n", __func__);
- } else {
- free_irq(irq, NULL);
- }
- return error;
-}
-
-static struct da8xx_ohci_root_hub omapl138_hawk_usb11_pdata = {
- .set_power = hawk_usb_set_power,
- .get_power = hawk_usb_get_power,
- .get_oci = hawk_usb_get_oci,
- .ocic_notify = hawk_usb_ocic_notify,
- /* TPS2087 switch @ 5V */
- .potpgt = (3 + 1) / 2, /* 3 ms max */
-};
-
-static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id)
-{
- hawk_usb_ocic_handler(&omapl138_hawk_usb11_pdata);
- return IRQ_HANDLED;
-}
-
static __init void omapl138_hawk_usb_init(void)
{
int ret;
- ret = davinci_cfg_reg_list(da850_hawk_usb11_pins);
- if (ret) {
- pr_warn("%s: USB 1.1 PinMux setup failed: %d\n", __func__, ret);
- return;
- }
-
/* USB_REFCLKIN is not used. */
ret = da8xx_register_usb20_phy_clk(false);
if (ret)
@@ -266,34 +197,11 @@ static __init void omapl138_hawk_usb_init(void)
pr_warn("%s: USB PHY registration failed: %d\n",
__func__, ret);
- ret = gpio_request_one(DA850_USB1_VBUS_PIN,
- GPIOF_DIR_OUT, "USB1 VBUS");
- if (ret < 0) {
- pr_err("%s: failed to request GPIO for USB 1.1 port "
- "power control: %d\n", __func__, ret);
- return;
- }
-
- ret = gpio_request_one(DA850_USB1_OC_PIN,
- GPIOF_DIR_IN, "USB1 OC");
- if (ret < 0) {
- pr_err("%s: failed to request GPIO for USB 1.1 port "
- "over-current indicator: %d\n", __func__, ret);
- goto usb11_setup_oc_fail;
- }
-
- ret = da8xx_register_usb11(&omapl138_hawk_usb11_pdata);
- if (ret) {
+ ret = da8xx_register_usb11();
+ if (ret)
pr_warn("%s: USB 1.1 registration failed: %d\n", __func__, ret);
- goto usb11_setup_fail;
- }
return;
-
-usb11_setup_fail:
- gpio_free(DA850_USB1_OC_PIN);
-usb11_setup_oc_fail:
- gpio_free(DA850_USB1_VBUS_PIN);
}
static __init void omapl138_hawk_init(void)
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index 38d932e..93d2db3 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -94,7 +94,7 @@
int da8xx_register_usb11_phy_clk(bool use_usb_refclkin);
int da8xx_register_usb_phy(void);
int da8xx_register_usb20(unsigned mA, unsigned potpgt);
-int da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata);
+int da8xx_register_usb11(void);
int da8xx_register_emac(void);
int da8xx_register_uio_pruss(void);
int da8xx_register_lcdc(struct da8xx_lcdc_platform_data *pdata);
diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
index 982e105..7a9af216 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -337,8 +337,7 @@ int __init da8xx_register_usb20(unsigned int mA, unsigned int potpgt)
.resource = da8xx_usb11_resources,
};
-int __init da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata)
+int __init da8xx_register_usb11(void)
{
- da8xx_usb11_device.dev.platform_data = pdata;
return platform_device_register(&da8xx_usb11_device);
}
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 5585d9e..f4bda4d 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -39,6 +39,8 @@ static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq,
static struct clk *usb11_clk;
static struct phy *usb11_phy;
+static struct regulator *vbus_reg;
+struct notifier_block nb;
/* Over-current indicator change flag */
static int ocic_flag;
@@ -76,22 +78,57 @@ static void ohci_da8xx_disable(void)
clk_disable_unprepare(usb11_clk);
}
-/*
- * Handle the port over-current indicator change.
- */
-static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub)
+static int ohci_da8xx_set_power(int on)
+{
+ int ret = 0;
+
+ if (!vbus_reg)
+ return 0;
+
+ if (on) {
+ ret = regulator_enable(vbus_reg);
+ if (ret)
+ pr_err("fail to enable regulator: %d\n", ret);
+ } else {
+ ret = regulator_disable(vbus_reg);
+ if (ret)
+ pr_err("fail to disable regulator: %d\n", ret);
+ }
+
+ return ret;
+}
+
+static int ohci_da8xx_get_power(void)
+{
+ if (!vbus_reg)
+ return 1;
+
+ return regulator_is_enabled(vbus_reg);
+}
+
+static int ohci_da8xx_get_oci(void)
+{
+ if (regulator_get_mode(vbus_reg) == REGULATOR_MODE_OVERCURRENT)
+ return 1;
+
+ return 0;
+}
+
+static int ohci_da8xx_regulator_event(struct notifier_block *nb,
+ unsigned long event, void *data)
{
- ocic_flag = 1;
+ if (event & REGULATOR_EVENT_OVER_CURRENT) {
+ ocic_flag = 1;
+ if (ohci_da8xx_get_oci())
+ ohci_da8xx_set_power(0);
+ }
- /* Once over-current is detected, the port needs to be powered down */
- if (hub->get_oci() > 0)
- hub->set_power(0);
+ return 0;
}
static int ohci_da8xx_reset(struct usb_hcd *hcd)
{
struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
int result;
u32 rh_a;
@@ -121,16 +158,14 @@ static int ohci_da8xx_reset(struct usb_hcd *hcd)
* the correct hub descriptor...
*/
rh_a = ohci_readl(ohci, &ohci->regs->roothub.a);
- if (hub->set_power) {
+
+ if (vbus_reg) {
rh_a &= ~RH_A_NPS;
rh_a |= RH_A_PSM;
- }
- if (hub->get_oci) {
rh_a &= ~RH_A_NOCP;
rh_a |= RH_A_OCPM;
}
- rh_a &= ~RH_A_POTPGT;
- rh_a |= hub->potpgt << 24;
+
ohci_writel(ohci, rh_a, &ohci->regs->roothub.a);
return result;
@@ -163,7 +198,6 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
u16 wIndex, char *buf, u16 wLength)
{
struct device *dev = hcd->self.controller;
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
int temp;
switch (typeReq) {
@@ -177,11 +211,11 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp = roothub_portstatus(hcd_to_ohci(hcd), wIndex - 1);
/* The port power status (PPS) bit defaults to 1 */
- if (hub->get_power && hub->get_power() == 0)
+ if (ohci_da8xx_get_power() == 0)
temp &= ~RH_PS_PPS;
/* The port over-current indicator (POCI) bit is always 0 */
- if (hub->get_oci && hub->get_oci() > 0)
+ if (ohci_da8xx_get_oci() > 0)
temp |= RH_PS_POCI;
/* The over-current indicator change (OCIC) bit is 0 too */
@@ -206,19 +240,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex, "POWER");
- if (!hub->set_power)
- return -EPIPE;
-
- return hub->set_power(temp) ? -EPIPE : 0;
+ return ohci_da8xx_set_power(temp) ? -EPIPE : 0;
case USB_PORT_FEAT_C_OVER_CURRENT:
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex,
"C_OVER_CURRENT");
- if (temp)
- ocic_flag = 1;
- else
- ocic_flag = 0;
+ ocic_flag = temp;
return 0;
}
}
@@ -231,14 +259,10 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
static int ohci_da8xx_probe(struct platform_device *pdev)
{
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
struct usb_hcd *hcd;
struct resource *mem;
int error, irq;
- if (hub == NULL)
- return -ENODEV;
-
hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev,
dev_name(&pdev->dev));
if (!hcd)
@@ -258,6 +282,22 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
return PTR_ERR(usb11_phy);
}
+ vbus_reg = devm_regulator_get(&pdev->dev, "vbus");
+ if (IS_ERR(vbus_reg)) {
+ if (PTR_ERR(vbus_reg) != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Failed to get regulator.\n");
+ return PTR_ERR(vbus_reg);
+ }
+
+ if (vbus_reg) {
+ nb.notifier_call = ohci_da8xx_regulator_event;
+ error = devm_regulator_register_notifier(vbus_reg, &nb);
+ if (error) {
+ dev_err(&pdev->dev,
+ "Could not register regulator notifier\n");
+ return error;
+ }
+ }
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
hcd->regs = devm_ioremap_resource(&pdev->dev, mem);
@@ -281,13 +321,7 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
device_wakeup_enable(hcd->self.controller);
- if (hub->ocic_notify) {
- error = hub->ocic_notify(ohci_da8xx_ocic_handler);
- if (!error)
- return 0;
- }
-
- usb_remove_hcd(hcd);
+ return 0;
err:
usb_put_hcd(hcd);
return error;
@@ -295,10 +329,8 @@ static int ohci_da8xx_probe(struct platform_device *pdev)
static int ohci_da8xx_remove(struct platform_device *pdev)
{
- struct usb_hcd *hcd = platform_get_drvdata(pdev);
- struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
+ struct usb_hcd *hcd = platform_get_drvdata(pdev);
- hub->ocic_notify(NULL);
usb_remove_hcd(hcd);
usb_put_hcd(hcd);
@@ -314,7 +346,6 @@ static int ohci_da8xx_suspend(struct platform_device *pdev,
bool do_wakeup = device_may_wakeup(&pdev->dev);
int ret;
-
if (time_before(jiffies, ohci->next_statechange))
msleep(5);
ohci->next_statechange = jiffies;
diff --git a/include/linux/platform_data/usb-davinci.h b/include/linux/platform_data/usb-davinci.h
index 3217fbe..58f4be0 100644
--- a/include/linux/platform_data/usb-davinci.h
+++ b/include/linux/platform_data/usb-davinci.h
@@ -11,25 +11,6 @@
#ifndef __ASM_ARCH_USB_H
#define __ASM_ARCH_USB_H
-struct da8xx_ohci_root_hub;
-
-typedef void (*da8xx_ocic_handler_t)(struct da8xx_ohci_root_hub *hub);
-
-/* Passed as the platform data to the OHCI driver */
-struct da8xx_ohci_root_hub {
- /* Switch the port power on/off */
- int (*set_power)(int on);
- /* Read the port power status */
- int (*get_power)(void);
- /* Read the port over-current indicator */
- int (*get_oci)(void);
- /* Over-current indicator change notification (pass NULL to disable) */
- int (*ocic_notify)(da8xx_ocic_handler_t handler);
-
- /* Time from power on to power good (in 2 ms units) */
- u8 potpgt;
-};
-
void davinci_setup_usb(unsigned mA, unsigned potpgt_ms);
#endif /* ifndef __ASM_ARCH_USB_H */
--
1.9.1
^ permalink raw reply related
* [PATCH/RFT v2 11/17] USB: OHCI: make ohci-da8xx a separate driver
From: ahaslam at baylibre.com @ 2016-10-24 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-1-ahaslam@baylibre.com>
From: Manjunath Goudar <manjunath.goudar@linaro.org>
Separate the Davinci OHCI host controller driver from ohci-hcd
host code so that it can be built as a separate driver module.
This work is part of enabling multi-platform kernels on ARM;
it would be nice to have in 3.11.
Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org>
---
drivers/usb/host/Kconfig | 2 +-
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ohci-da8xx.c | 185 +++++++++++++++++-------------------------
drivers/usb/host/ohci-hcd.c | 18 ----
4 files changed, 76 insertions(+), 130 deletions(-)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 83b6cec..642c6fe8 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -479,7 +479,7 @@ config USB_OHCI_HCD_OMAP3
OMAP3 and later chips.
config USB_OHCI_HCD_DAVINCI
- bool "OHCI support for TI DaVinci DA8xx"
+ tristate "OHCI support for TI DaVinci DA8xx"
depends on ARCH_DAVINCI_DA8XX
depends on USB_OHCI_HCD=y
select PHY_DA8XX_USB
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 6ef785b..2644537 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_USB_OHCI_HCD_AT91) += ohci-at91.o
obj-$(CONFIG_USB_OHCI_HCD_S3C2410) += ohci-s3c2410.o
obj-$(CONFIG_USB_OHCI_HCD_LPC32XX) += ohci-nxp.o
obj-$(CONFIG_USB_OHCI_HCD_PXA27X) += ohci-pxa27x.o
+obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index e98066d..5585d9e 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -11,16 +11,31 @@
* kind, whether express or implied.
*/
+#include <linux/clk.h>
+#include <linux/io.h>
#include <linux/interrupt.h>
#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
#include <linux/platform_device.h>
-#include <linux/clk.h>
#include <linux/phy/phy.h>
#include <linux/platform_data/usb-davinci.h>
+#include <linux/platform_device.h>
+#include <linux/usb.h>
+#include <linux/usb/hcd.h>
+#include <asm/unaligned.h>
-#ifndef CONFIG_ARCH_DAVINCI_DA8XX
-#error "This file is DA8xx bus glue. Define CONFIG_ARCH_DAVINCI_DA8XX."
-#endif
+#include "ohci.h"
+
+#define DRIVER_DESC "OHCI DA8XX driver"
+
+static const char hcd_name[] = "ohci-da8xx";
+
+static struct hc_driver __read_mostly ohci_da8xx_hc_driver;
+
+static int (*orig_ohci_hub_control)(struct usb_hcd *hcd, u16 typeReq,
+ u16 wValue, u16 wIndex, char *buf, u16 wLength);
+static int (*orig_ohci_hub_status_data)(struct usb_hcd *hcd, char *buf);
static struct clk *usb11_clk;
static struct phy *usb11_phy;
@@ -73,7 +88,7 @@ static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub)
hub->set_power(0);
}
-static int ohci_da8xx_init(struct usb_hcd *hcd)
+static int ohci_da8xx_reset(struct usb_hcd *hcd)
{
struct device *dev = hcd->self.controller;
struct da8xx_ohci_root_hub *hub = dev_get_platdata(dev);
@@ -93,7 +108,7 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
*/
ohci->num_ports = 1;
- result = ohci_init(ohci);
+ result = ohci_setup(hcd);
if (result < 0) {
ohci_da8xx_disable();
return result;
@@ -121,30 +136,12 @@ static int ohci_da8xx_init(struct usb_hcd *hcd)
return result;
}
-static void ohci_da8xx_stop(struct usb_hcd *hcd)
-{
- ohci_stop(hcd);
- ohci_da8xx_disable();
-}
-
-static int ohci_da8xx_start(struct usb_hcd *hcd)
-{
- struct ohci_hcd *ohci = hcd_to_ohci(hcd);
- int result;
-
- result = ohci_run(ohci);
- if (result < 0)
- ohci_da8xx_stop(hcd);
-
- return result;
-}
-
/*
* Update the status data from the hub with the over-current indicator change.
*/
static int ohci_da8xx_hub_status_data(struct usb_hcd *hcd, char *buf)
{
- int length = ohci_hub_status_data(hcd, buf);
+ int length = orig_ohci_hub_status_data(hcd, buf);
/* See if we have OCIC flag set */
if (ocic_flag) {
@@ -226,66 +223,13 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
}
}
- return ohci_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
+ return orig_ohci_hub_control(hcd, typeReq, wValue,
+ wIndex, buf, wLength);
}
-static const struct hc_driver ohci_da8xx_hc_driver = {
- .description = hcd_name,
- .product_desc = "DA8xx OHCI",
- .hcd_priv_size = sizeof(struct ohci_hcd),
-
- /*
- * generic hardware linkage
- */
- .irq = ohci_irq,
- .flags = HCD_USB11 | HCD_MEMORY,
-
- /*
- * basic lifecycle operations
- */
- .reset = ohci_da8xx_init,
- .start = ohci_da8xx_start,
- .stop = ohci_da8xx_stop,
- .shutdown = ohci_shutdown,
-
- /*
- * managing i/o requests and associated device resources
- */
- .urb_enqueue = ohci_urb_enqueue,
- .urb_dequeue = ohci_urb_dequeue,
- .endpoint_disable = ohci_endpoint_disable,
-
- /*
- * scheduling support
- */
- .get_frame_number = ohci_get_frame,
-
- /*
- * root hub support
- */
- .hub_status_data = ohci_da8xx_hub_status_data,
- .hub_control = ohci_da8xx_hub_control,
-
-#ifdef CONFIG_PM
- .bus_suspend = ohci_bus_suspend,
- .bus_resume = ohci_bus_resume,
-#endif
- .start_port_reset = ohci_start_port_reset,
-};
-
/*-------------------------------------------------------------------------*/
-
-/**
- * usb_hcd_da8xx_probe - initialize DA8xx-based HCDs
- * Context: !in_interrupt()
- *
- * Allocates basic resources for this USB host controller, and
- * then invokes the start() method for the HCD associated with it
- * through the hotplug entry's driver_data.
- */
-static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
- struct platform_device *pdev)
+static int ohci_da8xx_probe(struct platform_device *pdev)
{
struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
struct usb_hcd *hcd;
@@ -295,6 +239,11 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
if (hub == NULL)
return -ENODEV;
+ hcd = usb_create_hcd(&ohci_da8xx_hc_driver, &pdev->dev,
+ dev_name(&pdev->dev));
+ if (!hcd)
+ return -ENOMEM;
+
usb11_clk = devm_clk_get(&pdev->dev, "usb11");
if (IS_ERR(usb11_clk)) {
if (PTR_ERR(usb11_clk) != -EPROBE_DEFER)
@@ -309,9 +258,6 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
return PTR_ERR(usb11_phy);
}
- hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
- if (!hcd)
- return -ENOMEM;
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
hcd->regs = devm_ioremap_resource(&pdev->dev, mem);
@@ -323,13 +269,12 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
hcd->rsrc_start = mem->start;
hcd->rsrc_len = resource_size(mem);
- ohci_hcd_init(hcd_to_ohci(hcd));
-
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
error = -ENODEV;
goto err;
}
+
error = usb_add_hcd(hcd, irq, 0);
if (error)
goto err;
@@ -348,35 +293,14 @@ static int usb_hcd_da8xx_probe(const struct hc_driver *driver,
return error;
}
-/**
- * usb_hcd_da8xx_remove - shutdown processing for DA8xx-based HCDs
- * @dev: USB Host Controller being removed
- * Context: !in_interrupt()
- *
- * Reverses the effect of usb_hcd_da8xx_probe(), first invoking
- * the HCD's stop() method. It is always called from a thread
- * context, normally "rmmod", "apmd", or something similar.
- */
-static inline void
-usb_hcd_da8xx_remove(struct usb_hcd *hcd, struct platform_device *pdev)
+static int ohci_da8xx_remove(struct platform_device *pdev)
{
+ struct usb_hcd *hcd = platform_get_drvdata(pdev);
struct da8xx_ohci_root_hub *hub = dev_get_platdata(&pdev->dev);
hub->ocic_notify(NULL);
usb_remove_hcd(hcd);
usb_put_hcd(hcd);
-}
-
-static int ohci_hcd_da8xx_drv_probe(struct platform_device *dev)
-{
- return usb_hcd_da8xx_probe(&ohci_da8xx_hc_driver, dev);
-}
-
-static int ohci_hcd_da8xx_drv_remove(struct platform_device *dev)
-{
- struct usb_hcd *hcd = platform_get_drvdata(dev);
-
- usb_hcd_da8xx_remove(hcd, dev);
return 0;
}
@@ -426,12 +350,16 @@ static int ohci_da8xx_resume(struct platform_device *dev)
}
#endif
+static const struct ohci_driver_overrides da8xx_overrides __initconst = {
+ .reset = ohci_da8xx_reset
+};
+
/*
* Driver definition to register with platform structure.
*/
static struct platform_driver ohci_hcd_da8xx_driver = {
- .probe = ohci_hcd_da8xx_drv_probe,
- .remove = ohci_hcd_da8xx_drv_remove,
+ .probe = ohci_da8xx_probe,
+ .remove = ohci_da8xx_remove,
.shutdown = usb_hcd_platform_shutdown,
#ifdef CONFIG_PM
.suspend = ohci_da8xx_suspend,
@@ -442,4 +370,39 @@ static int ohci_da8xx_resume(struct platform_device *dev)
},
};
+static int __init ohci_da8xx_init(void)
+{
+
+ if (usb_disabled())
+ return -ENODEV;
+
+ pr_info("%s: " DRIVER_DESC "\n", hcd_name);
+ ohci_init_driver(&ohci_da8xx_hc_driver, &da8xx_overrides);
+
+ /*
+ * The Davinci da8xx HW has some unusual quirks, which require
+ * da8xx-specific workarounds. We override certain hc_driver
+ * functions here to achieve that. We explicitly do not enhance
+ * ohci_driver_overrides to allow this more easily, since this
+ * is an unusual case, and we don't want to encourage others to
+ * override these functions by making it too easy.
+ */
+
+ orig_ohci_hub_control = ohci_da8xx_hc_driver.hub_control;
+ orig_ohci_hub_status_data = ohci_da8xx_hc_driver.hub_status_data;
+
+ ohci_da8xx_hc_driver.hub_status_data = ohci_da8xx_hub_status_data;
+ ohci_da8xx_hc_driver.hub_control = ohci_da8xx_hub_control;
+
+ return platform_driver_register(&ohci_hcd_da8xx_driver);
+}
+module_init(ohci_da8xx_init);
+
+static void __exit ohci_da8xx_cleanup(void)
+{
+ platform_driver_unregister(&ohci_hcd_da8xx_driver);
+}
+module_exit(ohci_da8xx_cleanup);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
MODULE_ALIAS("platform:ohci");
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 1700908..8de174a 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1219,11 +1219,6 @@ void ohci_init_driver(struct hc_driver *drv,
#define SA1111_DRIVER ohci_hcd_sa1111_driver
#endif
-#ifdef CONFIG_USB_OHCI_HCD_DAVINCI
-#include "ohci-da8xx.c"
-#define DAVINCI_PLATFORM_DRIVER ohci_hcd_da8xx_driver
-#endif
-
#ifdef CONFIG_USB_OHCI_HCD_PPC_OF
#include "ohci-ppc-of.c"
#define OF_PLATFORM_DRIVER ohci_hcd_ppc_of_driver
@@ -1303,19 +1298,9 @@ static int __init ohci_hcd_mod_init(void)
goto error_tmio;
#endif
-#ifdef DAVINCI_PLATFORM_DRIVER
- retval = platform_driver_register(&DAVINCI_PLATFORM_DRIVER);
- if (retval < 0)
- goto error_davinci;
-#endif
-
return retval;
/* Error path */
-#ifdef DAVINCI_PLATFORM_DRIVER
- platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER);
- error_davinci:
-#endif
#ifdef TMIO_OHCI_DRIVER
platform_driver_unregister(&TMIO_OHCI_DRIVER);
error_tmio:
@@ -1351,9 +1336,6 @@ static int __init ohci_hcd_mod_init(void)
static void __exit ohci_hcd_mod_exit(void)
{
-#ifdef DAVINCI_PLATFORM_DRIVER
- platform_driver_unregister(&DAVINCI_PLATFORM_DRIVER);
-#endif
#ifdef TMIO_OHCI_DRIVER
platform_driver_unregister(&TMIO_OHCI_DRIVER);
#endif
--
1.9.1
^ permalink raw reply related
* [PATCH/RFT v2 10/17] USB: da8xx: use flag instead of bitmask for over current change
From: ahaslam at baylibre.com @ 2016-10-24 16:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161024164634.4330-1-ahaslam@baylibre.com>
From: Axel Haslam <ahaslam@baylibre.com>
The da8xx ohci has only one port connected. In hub_control we
check that the port is 1, or else we bail out early. There is no point
in passing as argument the port number, if we know, and check that it
will always be 1.
Simplify functions and callbacks, by removing the port parameter, and
converting the over current indicatior change form a mask to a flag.
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
arch/arm/mach-davinci/board-da830-evm.c | 8 ++++----
arch/arm/mach-davinci/board-omapl138-hawk.c | 8 ++++----
drivers/usb/host/ohci-da8xx.c | 29 ++++++++++++++---------------
include/linux/platform_data/usb-davinci.h | 9 ++++-----
4 files changed, 26 insertions(+), 28 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
index b33fc6b..d12fcf5 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -50,18 +50,18 @@
static da8xx_ocic_handler_t da830_evm_usb_ocic_handler;
-static int da830_evm_usb_set_power(unsigned port, int on)
+static int da830_evm_usb_set_power(int on)
{
gpio_set_value(ON_BD_USB_DRV, on);
return 0;
}
-static int da830_evm_usb_get_power(unsigned port)
+static int da830_evm_usb_get_power(void)
{
return gpio_get_value(ON_BD_USB_DRV);
}
-static int da830_evm_usb_get_oci(unsigned port)
+static int da830_evm_usb_get_oci(void)
{
return !gpio_get_value(ON_BD_USB_OVC);
}
@@ -100,7 +100,7 @@ static int da830_evm_usb_ocic_notify(da8xx_ocic_handler_t handler)
static irqreturn_t da830_evm_usb_ocic_irq(int irq, void *dev_id)
{
- da830_evm_usb_ocic_handler(&da830_evm_usb11_pdata, 1);
+ da830_evm_usb_ocic_handler(&da830_evm_usb11_pdata);
return IRQ_HANDLED;
}
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
index c3ab7ea..1d31f45 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -189,18 +189,18 @@ static __init void omapl138_hawk_mmc_init(void)
-1
};
-static int hawk_usb_set_power(unsigned port, int on)
+static int hawk_usb_set_power(int on)
{
gpio_set_value(DA850_USB1_VBUS_PIN, on);
return 0;
}
-static int hawk_usb_get_power(unsigned port)
+static int hawk_usb_get_power(void)
{
return gpio_get_value(DA850_USB1_VBUS_PIN);
}
-static int hawk_usb_get_oci(unsigned port)
+static int hawk_usb_get_oci(void)
{
return !gpio_get_value(DA850_USB1_OC_PIN);
}
@@ -237,7 +237,7 @@ static int hawk_usb_ocic_notify(da8xx_ocic_handler_t handler)
static irqreturn_t omapl138_hawk_usb_ocic_irq(int irq, void *dev_id)
{
- hawk_usb_ocic_handler(&omapl138_hawk_usb11_pdata, 1);
+ hawk_usb_ocic_handler(&omapl138_hawk_usb11_pdata);
return IRQ_HANDLED;
}
diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c
index 3656d7c..e98066d 100644
--- a/drivers/usb/host/ohci-da8xx.c
+++ b/drivers/usb/host/ohci-da8xx.c
@@ -25,8 +25,8 @@
static struct clk *usb11_clk;
static struct phy *usb11_phy;
-/* Over-current indicator change bitmask */
-static volatile u16 ocic_mask;
+/* Over-current indicator change flag */
+static int ocic_flag;
static int ohci_da8xx_enable(void)
{
@@ -64,14 +64,13 @@ static void ohci_da8xx_disable(void)
/*
* Handle the port over-current indicator change.
*/
-static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub,
- unsigned port)
+static void ohci_da8xx_ocic_handler(struct da8xx_ohci_root_hub *hub)
{
- ocic_mask |= 1 << port;
+ ocic_flag = 1;
/* Once over-current is detected, the port needs to be powered down */
- if (hub->get_oci(port) > 0)
- hub->set_power(port, 0);
+ if (hub->get_oci() > 0)
+ hub->set_power(0);
}
static int ohci_da8xx_init(struct usb_hcd *hcd)
@@ -147,8 +146,8 @@ static int ohci_da8xx_hub_status_data(struct usb_hcd *hcd, char *buf)
{
int length = ohci_hub_status_data(hcd, buf);
- /* See if we have OCIC bit set on port 1 */
- if (ocic_mask & (1 << 1)) {
+ /* See if we have OCIC flag set */
+ if (ocic_flag) {
dev_dbg(hcd->self.controller, "over-current indicator change "
"on port 1\n");
@@ -181,15 +180,15 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
temp = roothub_portstatus(hcd_to_ohci(hcd), wIndex - 1);
/* The port power status (PPS) bit defaults to 1 */
- if (hub->get_power && hub->get_power(wIndex) == 0)
+ if (hub->get_power && hub->get_power() == 0)
temp &= ~RH_PS_PPS;
/* The port over-current indicator (POCI) bit is always 0 */
- if (hub->get_oci && hub->get_oci(wIndex) > 0)
+ if (hub->get_oci && hub->get_oci() > 0)
temp |= RH_PS_POCI;
/* The over-current indicator change (OCIC) bit is 0 too */
- if (ocic_mask & (1 << wIndex))
+ if (ocic_flag)
temp |= RH_PS_OCIC;
put_unaligned(cpu_to_le32(temp), (__le32 *)buf);
@@ -213,16 +212,16 @@ static int ohci_da8xx_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
if (!hub->set_power)
return -EPIPE;
- return hub->set_power(wIndex, temp) ? -EPIPE : 0;
+ return hub->set_power(temp) ? -EPIPE : 0;
case USB_PORT_FEAT_C_OVER_CURRENT:
dev_dbg(dev, "%sPortFeature(%u): %s\n",
temp ? "Set" : "Clear", wIndex,
"C_OVER_CURRENT");
if (temp)
- ocic_mask |= 1 << wIndex;
+ ocic_flag = 1;
else
- ocic_mask &= ~(1 << wIndex);
+ ocic_flag = 0;
return 0;
}
}
diff --git a/include/linux/platform_data/usb-davinci.h b/include/linux/platform_data/usb-davinci.h
index 0926e99..3217fbe 100644
--- a/include/linux/platform_data/usb-davinci.h
+++ b/include/linux/platform_data/usb-davinci.h
@@ -13,17 +13,16 @@
struct da8xx_ohci_root_hub;
-typedef void (*da8xx_ocic_handler_t)(struct da8xx_ohci_root_hub *hub,
- unsigned port);
+typedef void (*da8xx_ocic_handler_t)(struct da8xx_ohci_root_hub *hub);
/* Passed as the platform data to the OHCI driver */
struct da8xx_ohci_root_hub {
/* Switch the port power on/off */
- int (*set_power)(unsigned port, int on);
+ int (*set_power)(int on);
/* Read the port power status */
- int (*get_power)(unsigned port);
+ int (*get_power)(void);
/* Read the port over-current indicator */
- int (*get_oci)(unsigned port);
+ int (*get_oci)(void);
/* Over-current indicator change notification (pass NULL to disable) */
int (*ocic_notify)(da8xx_ocic_handler_t handler);
--
1.9.1
^ 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