* Re: [PATCH 2/4] clk: samsung: Remove Exynos4415 driver (SoC not supported anymore)
From: Sylwester Nawrocki @ 2017-01-16 10:32 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-clk
Cc: Mark Rutland, devicetree, linux-samsung-soc,
Javier Martinez Canillas, Seung-Woo Kim, Michael Turquette,
Stephen Boyd, Tomasz Figa, dri-devel, linux-kernel, Chanwoo Choi,
Kyungmin Park, Rob Herring, Kukjin Kim, linux-gpio,
linux-arm-kernel
In-Reply-To: <20170114123642.15581-3-krzk@kernel.org>
On 01/14/2017 01:36 PM, Krzysztof Kozlowski wrote:
> Support for Exynos4415 is going away because there are no internal nor
> external users.
>
> Since commit 46dcf0ff0de3 ("ARM: dts: exynos: Remove exynos4415.dtsi"),
> the platform cannot be instantiated so remove also the drivers.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Applied, thanks.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH] pcie: ti: Provide patch to force GEN1 PCIe operation
From: Lukasz Majewski @ 2017-01-16 10:34 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Joao Pinto, jingoohan1@gmail.com, Bjorn Helgaas, Rob Herring,
Mark Rutland, linux-omap, linux-pci, devicetree, linux-kernel
In-Reply-To: <587C9CE6.8040001@ti.com>
Dear All,
> + Joao, Jingoo
>
> Hi,
>
> On Monday 16 January 2017 03:01 PM, Lukasz Majewski wrote:
> > Hi Kishon,
> >
> >> Hi Łukasz,
> >>
> >> On Monday 16 January 2017 12:19 PM, Lukasz Majewski wrote:
> >>> Hi Kishon,
> >>>
> >>>> Hi,
> >>>>
> >>>> On Sunday 15 January 2017 06:49 PM, Lukasz Majewski wrote:
> >>>>> Some devices (due to e.g. bad PCIe signal integrity) require to
> >>>>> run with forced GEN1 speed on PCIe bus.
> >>>>>
> >>>>> This patch changes the speed explicitly on dra7 based devices
> >>>>> when proper device tree attribute is defined for the PCIe
> >>>>> controller.
> >>>>>
> >>>>> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> >>>>
> >>>> Bjorn has already queued a patch to do the same thing
> >>>> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-dra7xx
> >>>
> >>> It seems like Bjorn only modifies CAP registers.
> >>
> >> The patch also modifies the LNKCTL2 register.
> >>>
> >>> He also needs to change register with 0x080C offset to actually
> >>> ( PCIECTRL_PL_WIDTH_SPEED_CTL )
> >>
> >> This bit is used to initiate speed change (after the link is
> >> initialized in GEN1). Resetting the bit (like what you have done
> >> here) prevents speed change.
> >
> > This is strange, but e2e advised me to do things as I did in the
> > patch to _force_ GEN1 operation on PCIe2 port [1] (AM5728)
> >
> > Link:
> > [1] https://e2e.ti.com/support/arm/sitara_arm/f/791/t/566421
> >
> > Both patches modify 0x5180 007C register to set GEN1 capability
> > (PCI_EXP_LNKCAP_SLS_2_5GB)
> >
> > The problem is with second register (in your patch):
> >
> > From SPRUHZ6G TRM:
> >
> > PCIECTRL_EP_DBICS_LNK_CAS_2 (0x5180 00A0)
> > - TRGT_LINK_SPEED (Reset 0x1) - "Target Link Speed" - no more
> > description in TRM
> >
> > It is set to PCI_EXP_LNKCAP_SLS_2_5GB = 0x1, which is the same as
> > default /reset value.
>
> The default value is 0x2 (or else none of the cards would have
> enumerated in GEN2)
SPRUHZ6G – October 2014 – Revised June 2016 - page 6313. Maybe TRM is
not up to date?
> >
> >
> > Could you clarify which way to _force_ PCIe GEN1 operation is
> > correct? Mine shows differences in lspci output (as posted in [1]).
>
> You'll see the difference even with the patch in Bjorn's tree ;-)
:-)
The details of my test cases and output are in the post [1].
>
> I think these are 2 different approaches to keep the link at GEN1.
> Joao or Jingoo, do you have any suggestion here?
Please read through thread [1]
[1] https://e2e.ti.com/support/arm/sitara_arm/f/791/t/566421
>
> >
> >>
> >> IMO the better way is to set the LNKCTL2 to GEN1 instead of hacking
> >> the IP register.
> >
> > From the original patch description:
> >
> > "Add support to force Root Complex to work in GEN1 mode if so
> > desired, but don't force GEN1 mode on any board just yet."
> >
> > Are there any (floating around) patches allowing forcing GEN1
> > operation on any board (I would like to reuse/port them to my
> > current solution)?
>
> For setting to GEN1 mode, "max-link-speed" should be set to 1 in dt
> with the patch in Bjorn's tree.
Ah.... ok.
>
> Thanks
> Kishon
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: usb: add DT binding for s3c2410 USB device controller
From: Felipe Balbi @ 2017-01-16 10:34 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Sergio Prado
In-Reply-To: <1484172150-32075-2-git-send-email-sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 344 bytes --]
Hi,
Sergio Prado <sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org> writes:
> Adds the device tree bindings description for Samsung S3C2410 and
> compatible USB device controller.
>
> Signed-off-by: Sergio Prado <sergio.prado-1e4yhPs3/ABSwrhanM7KvQ@public.gmane.org>
without Ack from DT folks, I can't take this.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define
From: Geert Uytterhoeven @ 2017-01-16 10:36 UTC (permalink / raw)
To: Chris Brandt
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-Renesas,
Wolfram Sang
In-Reply-To: <SG2PR06MB11659339E4A08373D3626BB28A780-ESzmfEwOt/xoAsOJh7vwSm0DtJ1/0DrXvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
Hi Chris,
CC Woflram
On Fri, Jan 13, 2017 at 6:56 PM, Chris Brandt <Chris.Brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> On Friday, January 13, 2017, Geert Uytterhoeven wrote:
>> > In sh_mobile_sdhi.c, can we change sh_mobile_sdhi_probe() so that if
>> > there are 2 clocks specified (in DT or platform data), it
>> > automatically enables the 2nd clock (forever) and just uses the 1st
>> clock as the on/off clock?
>>
>> Of course the driver can handle the second interrupt, if you update the
>> binding, and add support code for that...
>
> Of course my idea is that I would only have to update the bindings for
> RZ/A1...not any other device.
>
> My only question is, today sh_mobile_sdhi.c uses this:
>
> priv->clk = devm_clk_get(&pdev->dev, NULL);
>
> to get the clock. But if there is a 2nd clock...how do I know the
> string id name to look to replace NULL with??
That should be specified in the bindings. "NULL" will get you the first clock.
> Or...for the RZ/A1 dtsi, should I just give the 2 clocks names:
>
> clocks = <&mstp12_clks R7S72100_CLK_SDHI00,
> &mstp12_clks R7S72100_CLK_SDHI01>;
> clock-names = "core", "cd";
Something like that.
If devm_clk_get(&pdev->dev, "core") fails, you can assume a single clock
and retry with NULL.
> and then in the code do:
>
> struct *cd_clk;
> cd_clk = devm_clk_get(&pdev->dev, "cd");
> if (cd_clk) {
> clk_prepare_enable(cd_clk);
> }
>
> (this simple 1-line fix patch is getting a lot more complicated)
Disclaimer: I don't know how/if the SDHI core manages clocks, and may
interfere. Adding Wolfram.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 0/4] arm64: arch_timer: Add workaround for hisilicon-161601 erratum
From: Ding Tianhong @ 2017-01-16 10:37 UTC (permalink / raw)
To: Marc Zyngier, catalin.marinas, will.deacon, mark.rutland, oss,
devicetree, shawnguo, stuart.yoder, linux-arm-kernel, linuxarm
In-Reply-To: <06fd3327-f451-bc0d-b80c-4200bc9fadaa@huawei.com>
On 2017/1/12 21:24, Ding Tianhong wrote:
>
> On 2017/1/12 17:11, Marc Zyngier wrote:
>> On 12/01/17 04:23, Ding Tianhong wrote:
>>> Hi Marc:
>>>
>>> How about this v7, if any suggestions very grateful.
>>
>> It's been less than 5 days since you posted this. I'll get to it once I
>> finish reviewing all the other patches that are sitting in the queue
>> right before yours.
>>
>
> Ok and sorry for the noisy.
>
Hi Marc：
After discussion with the chip developer, we decide to update the erratum id for this bug, so I will resend a new version
about this, if you has start to review this v7 patch set, I think I could wait until you have finished yet. :)
Thanks
Ding
> Thanks
> Ding
>
>> Thanks,
>>
>> M.
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4] ARM64: dts: meson-gx: Add reserved memory zone and usable memory range
From: Neil Armstrong @ 2017-01-16 10:39 UTC (permalink / raw)
To: Andreas Färber, Kevin Hilman
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, xypron.glpk-Mmb7MZpHnFY,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, carlo-KA+7E9HrN00dnm+yROfE0A,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <d3a00788-c3c1-4b10-90bf-2a8bc6a138f8-l3A5Bk7waGM@public.gmane.org>
On 01/15/2017 03:43 PM, Andreas Färber wrote:
> Am 13.01.2017 um 21:03 schrieb Kevin Hilman:
>> Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:
>>
>>> The Amlogic Meson GXBB/GXL/GXM secure monitor uses part of the memory space,
>>> this patch adds this reserved zone and redefines the usable memory range.
>>>
>>> The memory node is also moved from the dtsi files into the proper dts files
>>> to handle variants memory sizes.
>>>
>>> This patch also fixes the memory sizes for the following platforms :
>>> - gxl-s905x-p212 : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
>>> - gxm-s912-q201 : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
>>> - gxl-s905d-p231 : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
>>> - gxl-nexbox-a95x : 1GiB instead of 2GiB, a proper 2GiB dts should be pushed
>>>
>>> Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>
>> Queued for v4.10-rc.
>
> What is the motivation for this change? I have a local U-Boot patch to
> detect the amount of memory available as done downstream, but U-Boot
> only updates the reg property that you seem to be abandoning here...
>
> So for devices that come in multiple RAM configurations - like R-Box Pro
> - this would require separate .dts files now! This looks very wrong to
> me, especially since I am not aware of other platforms doing the same.
> Instead, there's memory reservations for top and bottom done in U-Boot
> for reg, plus reserved-memory nodes for anything in the middle.
>
> Another thing to consider is that uEFI boot (bootefi) handles memory
> reservation differently yet again, on the bootloader level. I have had
> that working fine on Odroid-C2 and Vega S95.
>
> So if there's no bug this is fixing (none mentioned in commit message) I
> strongly object to this patch.
>
> Regards,
> Andreas
>
Hi Andreas,
Like I replied of my RFT patch :
I really disagree about relying on any work or properties added by any bootloader here, Amlogic SoCs has
a lot of u-boot versions in the field, and the Odroid-C2 is part of this.
Even if Odroid-c2 is in mainline U-Boot or not, the mainline Linux kernel should work using
any U-boot version even with the one provided by Amlogic on their openlinux distribution channel.
Handling multiple RAM configuration is another story, and the Arm-Soc and DT maintainers should give us
their advices.
Actually there is a severe bug fixed here that cause a huge crash if such memory is not reserved while
running stock u-boot version on various shipped products and Amlogic's own development boards.
The bug is easily triggered by running :
# stress --vm 4 --vm-bytes 128M --timeout 10s &
[ 46.937975] Bad mode in Error handler detected on CPU1, code 0xbf000000 -- SError
...
[ 47.058536] Internal error: Attempting to execute userspace memory: 8600000f [#3] PREEMPT SMP
...
Note this is a fix targeted for 4.10 to make the system stable and various users reported some severe
crash now the system has more drivers and read-world use-cases are running on Amlogic SoCs.
Please feel free to push whatever changes that makes this memory reservation more coherent for 4.11,
and respect the behavior of already shipped u-boot version and mainline U-Boot, UEFI, whatever...
Neil
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 1/2] of: base: add support to find the level of the last cache
From: Sudeep Holla @ 2017-01-16 10:40 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Sudeep Holla, Rob Herring, Catalin Marinas, Will Deacon,
devicetree, linux-kernel, Tan Xiaojun, Mark Rutland
It is useful to have helper function just to get the number of cache
levels for a given logical cpu. We can obtain the same by just checking
the level at which the last cache is present. This patch adds support
to find the level of the last cache for a given cpu.
It will be used on ARM64 platform where the device tree provides the
information for the additional non-architected/transparent/external
last level caches that are not integrated with the processors.
Cc: Mark Rutland <mark.rutland@arm.com>
Suggested-by: Rob Herring <robh+dt@kernel.org>
Acked-by: Rob Herring <robh+dt@kernel.org>
Tested-by: Tan Xiaojun <tanxiaojun@huawei.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/of/base.c | 26 ++++++++++++++++++++++++++
include/linux/of.h | 1 +
2 files changed, 27 insertions(+)
v2->v3:
- Dropped unnecessary pointer check
- Added Rob's Ack and Tan's Tested-by tags
v1->v2:
- Moved to using "cache-level" in the last level cache instead
of counting through all the nodes as suggested by Rob
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d4bea3c797d6..a30f541f0825 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -25,6 +25,7 @@
#include <linux/cpu.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/of_graph.h>
#include <linux/spinlock.h>
#include <linux/slab.h>
@@ -2268,6 +2269,31 @@ struct device_node *of_find_next_cache_node(const struct device_node *np)
}
/**
+ * of_find_last_cache_level - Find the level at which the last cache is
+ * present for the given logical cpu
+ *
+ * @cpu: cpu number(logical index) for which the last cache level is needed
+ *
+ * Returns the the level at which the last cache is present. It is exactly
+ * same as the total number of cache levels for the given logical cpu.
+ */
+int of_find_last_cache_level(unsigned int cpu)
+{
+ int cache_level = 0;
+ struct device_node *prev = NULL, *np = of_cpu_device_node_get(cpu);
+
+ while (np) {
+ prev = np;
+ of_node_put(np);
+ np = of_find_next_cache_node(np);
+ }
+
+ of_property_read_u32(prev, "cache-level", &cache_level);
+
+ return cache_level;
+}
+
+/**
* of_graph_parse_endpoint() - parse common endpoint node properties
* @node: pointer to endpoint device_node
* @endpoint: pointer to the OF endpoint data structure
diff --git a/include/linux/of.h b/include/linux/of.h
index d72f01009297..21e6323de0f3 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -280,6 +280,7 @@ extern struct device_node *of_get_child_by_name(const struct device_node *node,
/* cache lookup */
extern struct device_node *of_find_next_cache_node(const struct device_node *);
+extern int of_find_last_cache_level(unsigned int cpu);
extern struct device_node *of_find_node_with_property(
struct device_node *from, const char *prop_name);
--
2.7.4
^ permalink raw reply related
* [PATCH v3 2/2] arm64: cacheinfo: add support to override cache levels via device tree
From: Sudeep Holla @ 2017-01-16 10:40 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Sudeep Holla, Rob Herring, Catalin Marinas, Will Deacon,
devicetree, linux-kernel, Tan Xiaojun, Mark Rutland
In-Reply-To: <1484563244-14743-1-git-send-email-sudeep.holla@arm.com>
The cache hierarchy can be identified through Cache Level ID(CLIDR)
architected system register. However in some cases it will provide
only the number of cache levels that are integrated into the processor
itself. In other words, it can't provide any information about the
caches that are external and/or transparent.
Some platforms require to export the information about all such external
caches to the userspace applications via the sysfs interface.
This patch adds support to override the cache levels using device tree
to take such external non-architected caches into account.
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Tested-by: Tan Xiaojun <tanxiaojun@huawei.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
arch/arm64/kernel/cacheinfo.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c
index 9617301f76b5..3f2250fc391b 100644
--- a/arch/arm64/kernel/cacheinfo.c
+++ b/arch/arm64/kernel/cacheinfo.c
@@ -84,7 +84,7 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
static int __init_cache_level(unsigned int cpu)
{
- unsigned int ctype, level, leaves;
+ unsigned int ctype, level, leaves, of_level;
struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
for (level = 1, leaves = 0; level <= MAX_CACHE_LEVEL; level++) {
@@ -97,6 +97,17 @@ static int __init_cache_level(unsigned int cpu)
leaves += (ctype == CACHE_TYPE_SEPARATE) ? 2 : 1;
}
+ of_level = of_find_last_cache_level(cpu);
+ if (level < of_level) {
+ /*
+ * some external caches not specified in CLIDR_EL1
+ * the information may be available in the device tree
+ * only unified external caches are considered here
+ */
+ leaves += (of_level - level);
+ level = of_level;
+ }
+
this_cpu_ci->num_levels = level;
this_cpu_ci->num_leaves = leaves;
return 0;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] mtd: spi-nor: don't claim mr25h40 to be JEDEC compatible
From: Uwe Kleine-König @ 2017-01-16 10:40 UTC (permalink / raw)
To: Mark Rutland
Cc: Geert Uytterhoeven, Masahiko Iwamoto, Jagan Teki, Marek Vasut,
Cyrille Pitchen, MTD Maling List, Sascha Hauer,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20170113194226.GH2472@leverpostej>
Hello,
On Fri, Jan 13, 2017 at 07:42:27PM +0000, Mark Rutland wrote:
> On Fri, Jan 13, 2017 at 07:42:34PM +0100, Geert Uytterhoeven wrote:
> > CC devicetree
thanks
> >
> > On Fri, Jan 13, 2017 at 10:35 AM, Uwe Kleine-König
> > <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> > > Commit edd0c8f4932d ("mtd: spi-nor: Add support for mr25h40") made it
> > > possible to use a mr25h40 by writing
> > >
> > > compatible = "mr25h40", "jedec,spi-nor";
> >
> > No vendor prefix?
> >
> > >
> > > in a device tree. This chip however isn't JEDEC compatible however, so
> > > change the chip string and add a compatible entry to bless
> > >
> > > compatible = "mr25h40-nonjedec";
> > >
> > > as the right way.
> >
> > This whole "-nonjedec" business looks wrong to me.
> > If the device is called "mr25h40", its compatible value should be
> > "everspin,mr25h40". Adding some (in)compatibility indicator violates the
> > spirit of compatible values, IMHO.
>
> Agreed on all counts.
>
> The compatible string should specify the vendor and device, any
> compliance details should either be known for that string or derived
> from other properties.
>
> IIUC this is following an existing pattern, which we should deprecate
> (retaining support for those strings so old DTBs work).
Looking at drivers/mtd/spi-nor/spi-nor.c there is in the spi_nor_ids
array:
...
{ "m25p05", INFO(0x202010, 0, 32 * 1024, 2, 0) },
...
{ "m25p05-nonjedec", INFO(0, 0, 32 * 1024, 2, 0) },
and similar entries for the other M25P members. So I guess these chips
couldn't do JEDEC at the beginning, then got feature updates but no new
name. So "m25p05-nonjedec" is fine as compatibility string?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] Broadcom USB DRD Phy driver for Northstar2
From: Raviteja Garimella @ 2017-01-16 10:51 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Rob Herring, Mark Rutland, Ray Jui, Scott Branden, Jon Mason,
Catalin Marinas, Will Deacon, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, BCM Kernel Feedback,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <587C90E2.5020105-l0cyMroinI0@public.gmane.org>
Hi Kishon,
On Mon, Jan 16, 2017 at 2:52 PM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> wrote:
> Hi,
>
> On Wednesday 30 November 2016 11:25 AM, Raviteja Garimella wrote:
>> This is driver for USB DRD Phy used in Broadcom's Northstar2
>> SoC. The phy can be configured to be in Device mode or Host
>> mode based on the type of cable connected to the port. The
>> driver registers to extcon framework to get appropriate
>> connect events for Host/Device cables connect/disconnect
>> states based on VBUS and ID interrupts.
>>
>> Signed-off-by: Raviteja Garimella <raviteja.garimella-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> ---
>> drivers/phy/Kconfig | 13 +
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-bcm-ns2-usbdrd.c | 587 +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 601 insertions(+)
>> create mode 100644 drivers/phy/phy-bcm-ns2-usbdrd.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index fe00f91..b3b6a73 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -479,6 +479,19 @@ config PHY_CYGNUS_PCIE
>> Enable this to support the Broadcom Cygnus PCIe PHY.
>> If unsure, say N.
>>
>> +config PHY_NS2_USB_DRD
>> + tristate "Broadcom Northstar2 USB DRD PHY support"
>> + depends on OF && (ARCH_BCM_IPROC || COMPILE_TEST)
>> + select GENERIC_PHY
>> + select EXTCON
>> + default ARCH_BCM_IPROC
>> + help
>> + Enable this to support the Broadcom Northstar2 USB DRD PHY.
>> + This driver initializes the PHY in either HOST or DEVICE mode.
>> + The host or device configuration is read from device tree.
>> +
>> + If unsure, say N.
>> +
>> source "drivers/phy/tegra/Kconfig"
>>
>> config PHY_NS2_PCIE
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index a534cf5..b733ba2 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -58,5 +58,6 @@ obj-$(CONFIG_PHY_TUSB1210) += phy-tusb1210.o
>> obj-$(CONFIG_PHY_BRCM_SATA) += phy-brcm-sata.o
>> obj-$(CONFIG_PHY_PISTACHIO_USB) += phy-pistachio-usb.o
>> obj-$(CONFIG_PHY_CYGNUS_PCIE) += phy-bcm-cygnus-pcie.o
>> +obj-$(CONFIG_PHY_NS2_USB_DRD) += phy-bcm-ns2-usbdrd.o
>> obj-$(CONFIG_ARCH_TEGRA) += tegra/
>> obj-$(CONFIG_PHY_NS2_PCIE) += phy-bcm-ns2-pcie.o
>> diff --git a/drivers/phy/phy-bcm-ns2-usbdrd.c b/drivers/phy/phy-bcm-ns2-usbdrd.c
>> new file mode 100644
>> index 0000000..460040d
>> --- /dev/null
>> +++ b/drivers/phy/phy-bcm-ns2-usbdrd.c
>> @@ -0,0 +1,587 @@
>> +/*
>> + * Copyright (C) 2016 Broadcom
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/extcon.h>
>> +#include <linux/gpio.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/irq.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +#include <linux/workqueue.h>
>> +
>> +#define ICFG_DRD_AFE 0x0
>> +#define ICFG_MISC_STAT 0x18
>> +#define ICFG_DRD_P0CTL 0x1C
>> +#define ICFG_STRAP_CTRL 0x20
>> +#define ICFG_FSM_CTRL 0x24
>> +
>> +#define IDM_RST_BIT BIT(0)
>> +#define AFE_CORERDY_VDDC BIT(18)
>> +#define PHY_PLL_RESETB BIT(15)
>> +#define PHY_RESETB BIT(14)
>> +#define PHY_PLL_LOCK BIT(0)
>> +
>> +#define DRD_DEV_MODE BIT(20)
>> +#define OHCI_OVRCUR_POL BIT(11)
>> +#define ICFG_OFF_MODE BIT(6)
>> +#define PLL_LOCK_RETRY 1000
>> +
>> +#define EVT_DEVICE 0
>> +#define EVT_HOST 1
>> +#define EVT_IDLE 2
>> +
>> +#define DRD_HOST_MODE (BIT(2) | BIT(3))
>> +#define DRD_DEVICE_MODE (BIT(4) | BIT(5))
>> +#define DRD_HOST_VAL 0x803
>> +#define DRD_DEV_VAL 0x807
>> +#define GPIO_DELAY 20
>> +#define PHY_WQ_DELAY msecs_to_jiffies(600)
>> +
>> +struct ns2_phy_data;
>> +struct ns2_phy_driver {
>> + void __iomem *icfgdrd_regs;
>> + void __iomem *idmdrd_rst_ctrl;
>> + void __iomem *crmu_usb2_ctrl;
>> + void __iomem *usb2h_strap_reg;
>> + spinlock_t lock; /* spin lock for phy driver */
>> + bool host_mode;
>> + struct ns2_phy_data *data;
>> + struct extcon_specific_cable_nb extcon_dev;
>> + struct extcon_specific_cable_nb extcon_host;
>> + struct notifier_block host_nb;
>> + struct notifier_block dev_nb;
>> + struct delayed_work conn_work;
>> + struct extcon_dev *edev;
>> + struct gpio_desc *vbus_gpiod;
>> + struct gpio_desc *id_gpiod;
>> + int id_irq;
>> + int vbus_irq;
>> + unsigned long debounce_jiffies;
>> + struct delayed_work wq_extcon;
>> +};
>> +
>> +struct ns2_phy_data {
>> + struct ns2_phy_driver *driver;
>> + struct phy *phy;
>> + int new_state;
>> + bool poweron;
>> +};
>> +
>> +static const unsigned int usb_extcon_cable[] = {
>> + EXTCON_USB,
>> + EXTCON_USB_HOST,
>> + EXTCON_NONE,
>> +};
>> +
>> +static inline int pll_lock_stat(u32 usb_reg, int reg_mask,
>> + struct ns2_phy_driver *driver)
>> +{
>> + int retry = PLL_LOCK_RETRY;
>> + u32 val;
>> +
>> + do {
>> + udelay(1);
>> + val = readl(driver->icfgdrd_regs + usb_reg);
>> + if (val & reg_mask)
>> + return 0;
>> + } while (--retry > 0);
>> +
>> + return -EBUSY;
>> +}
>> +
>> +static int ns2_drd_phy_init(struct phy *phy)
>> +{
>> + struct ns2_phy_data *data = phy_get_drvdata(phy);
>> + struct ns2_phy_driver *driver = data->driver;
>> + unsigned long flags;
>> + u32 val;
>> +
>> + spin_lock_irqsave(&driver->lock, flags);
>> +
>> + val = readl(driver->icfgdrd_regs + ICFG_FSM_CTRL);
>> +
>> + if (data->new_state == EVT_HOST) {
>> + val &= ~DRD_DEVICE_MODE;
>> + val |= DRD_HOST_MODE;
>> + } else {
>> + val &= ~DRD_HOST_MODE;
>> + val |= DRD_DEVICE_MODE;
>> + }
>> + writel(val, driver->icfgdrd_regs + ICFG_FSM_CTRL);
>> +
>> + spin_unlock_irqrestore(&driver->lock, flags);
>> + return 0;
>> +}
>> +
>> +static int ns2_drd_phy_shutdown(struct phy *phy)
>> +{
>> + struct ns2_phy_data *data = phy_get_drvdata(phy);
>> + struct ns2_phy_driver *driver = data->driver;
>> + unsigned long flags;
>> + u32 val;
>> +
>> + spin_lock_irqsave(&driver->lock, flags);
>> + if (!data->poweron)
>> + goto exit;
>> +
>> + val = readl(driver->crmu_usb2_ctrl);
>> + val &= ~AFE_CORERDY_VDDC;
>> + writel(val, driver->crmu_usb2_ctrl);
>> +
>> + driver->host_mode = 0;
>> + val = readl(driver->crmu_usb2_ctrl);
>> + val &= ~DRD_DEV_MODE;
>> + writel(val, driver->crmu_usb2_ctrl);
>> +
>> + /* Disable Host and Device Mode */
>> + val = readl(driver->icfgdrd_regs + ICFG_FSM_CTRL);
>> + val &= ~(DRD_HOST_MODE | DRD_DEVICE_MODE | ICFG_OFF_MODE);
>> + writel(val, driver->icfgdrd_regs + ICFG_FSM_CTRL);
>> +
>> + data->poweron = 0;
>> +exit:
>> + spin_unlock_irqrestore(&driver->lock, flags);
>> + return 0;
>> +}
>> +
>> +static int ns2_drd_phy_poweron(struct phy *phy)
>> +{
>> + struct ns2_phy_data *data = phy_get_drvdata(phy);
>> + struct ns2_phy_driver *driver = data->driver;
>> + u32 extcon_event = data->new_state;
>> + unsigned long flags;
>> + int ret;
>> + u32 val;
>> +
>> + spin_lock_irqsave(&driver->lock, flags);
>> + if (extcon_event == EVT_DEVICE) {
>> + if (data->poweron)
>> + goto exit;
>> +
>> + writel(DRD_DEV_VAL, driver->icfgdrd_regs + ICFG_DRD_P0CTL);
>> +
>> + val = readl(driver->icfgdrd_regs + ICFG_FSM_CTRL);
>> + val &= ~(DRD_HOST_MODE | ICFG_OFF_MODE);
>> + val |= DRD_DEVICE_MODE;
>> + writel(val, driver->icfgdrd_regs + ICFG_FSM_CTRL);
>> +
>> + val = readl(driver->idmdrd_rst_ctrl);
>> + val &= ~IDM_RST_BIT;
>> + writel(val, driver->idmdrd_rst_ctrl);
>> +
>> + val = readl(driver->crmu_usb2_ctrl);
>> + val |= (AFE_CORERDY_VDDC | DRD_DEV_MODE);
>> + writel(val, driver->crmu_usb2_ctrl);
>> +
>> + /* Bring PHY and PHY_PLL out of Reset */
>> + val = readl(driver->crmu_usb2_ctrl);
>> + val |= (PHY_PLL_RESETB | PHY_RESETB);
>> + writel(val, driver->crmu_usb2_ctrl);
>> +
>> + ret = pll_lock_stat(ICFG_MISC_STAT, PHY_PLL_LOCK, driver);
>> + if (ret < 0) {
>> + dev_err(&phy->dev, "Phy PLL lock failed\n");
>> + goto err_shutdown;
>> + }
>> + } else {
>> + if (data->poweron && driver->host_mode)
>> + goto exit;
>> +
>> + writel(DRD_HOST_VAL, driver->icfgdrd_regs + ICFG_DRD_P0CTL);
>> +
>> + val = readl(driver->icfgdrd_regs + ICFG_FSM_CTRL);
>> + val &= ~(DRD_DEVICE_MODE | ICFG_OFF_MODE);
>> + val |= DRD_HOST_MODE;
>> + writel(val, driver->icfgdrd_regs + ICFG_FSM_CTRL);
>> +
>> + val = readl(driver->crmu_usb2_ctrl);
>> + val |= AFE_CORERDY_VDDC;
>> + writel(val, driver->crmu_usb2_ctrl);
>> +
>> + ret = pll_lock_stat(ICFG_MISC_STAT, PHY_PLL_LOCK, driver);
>> + if (ret < 0) {
>> + dev_err(&phy->dev, "Phy PLL lock failed\n");
>> + goto err_shutdown;
>> + }
>> +
>> + val = readl(driver->idmdrd_rst_ctrl);
>> + val &= ~IDM_RST_BIT;
>> + writel(val, driver->idmdrd_rst_ctrl);
>> +
>> + /* port over current Polarity */
>> + val = readl(driver->usb2h_strap_reg);
>> + val |= OHCI_OVRCUR_POL;
>> + writel(val, driver->usb2h_strap_reg);
>> +
>> + driver->host_mode = 1;
>> + }
>> +
>> + data->poweron = 1;
>> +exit:
>> + spin_unlock_irqrestore(&driver->lock, flags);
>> + return 0;
>> +
>> +err_shutdown:
>> + data->poweron = 1;
>> + spin_unlock_irqrestore(&driver->lock, flags);
>> + ns2_drd_phy_shutdown(phy);
>> + return ret;
>> +}
>> +
>> +static void connect_work(struct work_struct *work)
>> +{
>> + struct ns2_phy_driver *driver;
>> + struct ns2_phy_data *data;
>> + u32 extcon_event;
>> +
>> + driver = container_of(to_delayed_work(work),
>> + struct ns2_phy_driver, conn_work);
>> + data = driver->data;
>> + extcon_event = data->new_state;
>> +
>> + if (extcon_event == EVT_DEVICE || extcon_event == EVT_HOST) {
>> + ns2_drd_phy_init(data->phy);
>> + ns2_drd_phy_poweron(data->phy);
>> + } else if (extcon_event == EVT_IDLE) {
>> + ns2_drd_phy_shutdown(data->phy);
>> + }
>> +}
>> +
>> +static int drd_device_notify(struct notifier_block *self,
>> + unsigned long event, void *ptr)
>> +{
>> + struct ns2_phy_driver *driver = container_of(self,
>> + struct ns2_phy_driver, dev_nb);
>> +
>> + if (event) {
>> + pr_debug("Device connected\n");
>> + driver->data->new_state = EVT_DEVICE;
>> + schedule_delayed_work(&driver->conn_work, 0);
>> + } else {
>> + pr_debug("Device disconnected\n");
>> + driver->data->new_state = EVT_IDLE;
>> + schedule_delayed_work(&driver->conn_work, PHY_WQ_DELAY);
>> + }
>> +
>> + return NOTIFY_DONE;
>> +}
>> +
>> +static int drd_host_notify(struct notifier_block *self,
>> + unsigned long event, void *ptr)
>> +{
>> + struct ns2_phy_driver *driver = container_of(self,
>> + struct ns2_phy_driver, host_nb);
>> +
>> + if (event) {
>> + pr_debug("Host connected\n");
>> + driver->data->new_state = EVT_HOST;
>> + schedule_delayed_work(&driver->conn_work, 0);
>> + } else {
>> + pr_debug("Host disconnected\n");
>> + driver->data->new_state = EVT_IDLE;
>> + schedule_delayed_work(&driver->conn_work, PHY_WQ_DELAY);
>> + }
>> +
>> + return NOTIFY_DONE;
>> +}
>> +
>> +static void extcon_work(struct work_struct *work)
>> +{
>> + struct ns2_phy_driver *driver;
>> + int vbus;
>> + int id;
>> +
>> + driver = container_of(to_delayed_work(work),
>> + struct ns2_phy_driver, wq_extcon);
>> +
>> + id = gpiod_get_value_cansleep(driver->id_gpiod);
>> + vbus = gpiod_get_value_cansleep(driver->vbus_gpiod);
>> +
>> + if (!id && vbus) {
>> + extcon_set_cable_state_(driver->edev, EXTCON_USB_HOST, true);
>> + } else if (id && !vbus) {
>> + extcon_set_cable_state_(driver->edev, EXTCON_USB_HOST, false);
>> + extcon_set_cable_state_(driver->edev, EXTCON_USB, false);
>> + } else if (id && vbus) {
>> + extcon_set_cable_state_(driver->edev, EXTCON_USB, true);
>> + }
>> +}
>> +
>> +static irqreturn_t gpio_irq_handler(int irq, void *dev_id)
>> +{
>> + struct ns2_phy_driver *driver = dev_id;
>> +
>> + queue_delayed_work(system_power_efficient_wq, &driver->wq_extcon,
>> + driver->debounce_jiffies);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int register_extcon_notifier(struct ns2_phy_driver *phy_driver,
>> + struct device *dev)
>> +{
>> + struct extcon_dev *edev;
>> + int ret;
>> +
>> + phy_driver->host_nb.notifier_call = drd_host_notify;
>> + phy_driver->dev_nb.notifier_call = drd_device_notify;
>> +
>> + edev = phy_driver->edev;
>> +
>> + /* Register for device change notification */
>> + ret = extcon_register_notifier(edev, EXTCON_USB,
>> + &phy_driver->dev_nb);
>> + if (ret < 0) {
>> + dev_err(dev, "can't register extcon_dev for %s\n", edev->name);
>> + return ret;
>> + }
>> +
>> + /* Register for host change notification */
>> + ret = extcon_register_notifier(edev, EXTCON_USB_HOST,
>> + &phy_driver->host_nb);
>> + if (ret < 0) {
>> + dev_err(dev, "can't register extcon_dev for %s\n", edev->name);
>> + goto err_dev;
>> + }
>> +
>> + /* Check the device cable connect state */
>> + ret = extcon_get_cable_state_(edev, EXTCON_USB);
>> + if (ret < 0) {
>> + dev_err(dev, "can't get extcon_dev state for %s\n", edev->name);
>> + goto err_host;
>> + } else if (ret) {
>> + phy_driver->data->new_state = EVT_DEVICE;
>> + }
>> +
>> + /* Check the host cable connect state */
>> + ret = extcon_get_cable_state_(edev, EXTCON_USB_HOST);
>> + if (ret < 0) {
>> + dev_err(dev, "can't get extcon_dev state for %s\n", edev->name);
>> + goto err_host;
>> + } else if (ret) {
>> + phy_driver->data->new_state = EVT_HOST;
>> + }
>> +
>> + return 0;
>> +
>> +err_host:
>> + ret = extcon_unregister_notifier(edev, EXTCON_USB_HOST,
>> + &phy_driver->host_nb);
>> +err_dev:
>> + ret = extcon_unregister_notifier(edev, EXTCON_USB,
>> + &phy_driver->dev_nb);
>> + return ret;
>> +}
>> +
>> +static struct phy_ops ops = {
>> + .init = ns2_drd_phy_init,
>
> Is this really being used by any controller driver? Can you point me to the
> controller driver that is using this driver?
This will be used by Synopsys UDC that's integrated into Broadcom's Northstar2
and Cygnus SoC's. I am currently working on upstream review comments for the
same in a different patch series. The "amd5536udc" driver is being modified for
platform device support (as per the suggestions I received in reviews).
>
>> + .power_on = ns2_drd_phy_poweron,
>> + .power_off = ns2_drd_phy_shutdown,
>
> missing .owner.
Will fix this.
Thanks,
Ravi
>
> Thanks
> Kishon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2 1/3] dt: bindings: add documentation for zx2967 family reset controller
From: Baoyou Xie @ 2017-01-16 10:56 UTC (permalink / raw)
To: jun.nie, p.zabel, robh+dt, mark.rutland
Cc: linux-arm-kernel, devicetree, linux-kernel, shawnguo, baoyou.xie,
xie.baoyou, chen.chaokai, wang.qiang01
This patch adds dt-binding documentation for zx2967 family
reset controller.
Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
Reviewed-by: Shawn Guo <shawnguo@kernel.org>
---
.../devicetree/bindings/reset/zte,zx2967-reset.txt | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
create mode 100644 Documentation/devicetree/bindings/reset/zte,zx2967-reset.txt
diff --git a/Documentation/devicetree/bindings/reset/zte,zx2967-reset.txt b/Documentation/devicetree/bindings/reset/zte,zx2967-reset.txt
new file mode 100644
index 0000000..b015508
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/zte,zx2967-reset.txt
@@ -0,0 +1,20 @@
+ZTE zx2967 SoCs Reset Controller
+=======================================
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+Required properties:
+- compatible: should be one of the following.
+ * zte,zx296718-reset
+- reg: physical base address of the controller and length of memory mapped
+ region.
+- #reset-cells: must be 1.
+
+example:
+
+ reset: reset-controller@1461060 {
+ compatible = "zte,zx296718-reset";
+ reg = <0x01461060 0x8>;
+ #reset-cells = <1>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/3] MAINTAINERS: add zx2967 reset controller driver to ARM ZTE architecture
From: Baoyou Xie @ 2017-01-16 10:56 UTC (permalink / raw)
To: jun.nie, p.zabel, robh+dt, mark.rutland
Cc: linux-arm-kernel, devicetree, linux-kernel, shawnguo, baoyou.xie,
xie.baoyou, chen.chaokai, wang.qiang01
In-Reply-To: <1484564194-18530-1-git-send-email-baoyou.xie@linaro.org>
Add the zx2967 reset controller driver as maintained by ARM ZTE
architecture maintainers, as they're parts of the core IP.
Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2793808..08f8155 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1980,10 +1980,12 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
S: Maintained
F: arch/arm/mach-zx/
F: drivers/clk/zte/
+F: drivers/reset/reset-zx2967.c
F: drivers/soc/zte/
F: drivers/thermal/zx*
F: Documentation/devicetree/bindings/arm/zte.txt
F: Documentation/devicetree/bindings/clock/zx296702-clk.txt
+F: Documentation/devicetree/bindings/reset/zte,zx2967-reset.txt
F: Documentation/devicetree/bindings/soc/zte/
F: Documentation/devicetree/bindings/thermal/zx*
F: include/dt-bindings/soc/zx*.h
--
2.7.4
^ permalink raw reply related
* [PATCH v2 3/3] reset: zx2967: add reset controller driver for ZTE's zx2967 family
From: Baoyou Xie @ 2017-01-16 10:56 UTC (permalink / raw)
To: jun.nie, p.zabel, robh+dt, mark.rutland
Cc: linux-arm-kernel, devicetree, linux-kernel, shawnguo, baoyou.xie,
xie.baoyou, chen.chaokai, wang.qiang01
In-Reply-To: <1484564194-18530-1-git-send-email-baoyou.xie@linaro.org>
This patch adds reset controller driver for ZTE's zx2967 family.
Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
drivers/reset/Kconfig | 6 +++
drivers/reset/Makefile | 1 +
drivers/reset/reset-zx2967.c | 125 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 132 insertions(+)
create mode 100644 drivers/reset/reset-zx2967.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 172dc96..f4cdfe9 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -86,6 +86,12 @@ config RESET_UNIPHIER
Say Y if you want to control reset signals provided by System Control
block, Media I/O block, Peripheral Block.
+config RESET_ZX2967
+ bool "ZTE ZX2967 Reset Driver"
+ depends on ARCH_ZX || COMPILE_TEST
+ help
+ This enables the reset controller driver for ZTE's zx2967 family.
+
config RESET_ZYNQ
bool "ZYNQ Reset Driver" if COMPILE_TEST
default ARCH_ZYNQ
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 13b346e..2cd3f6c 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -13,4 +13,5 @@ obj-$(CONFIG_RESET_STM32) += reset-stm32.o
obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
+obj-$(CONFIG_RESET_ZX2967) += reset-zx2967.o
obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
diff --git a/drivers/reset/reset-zx2967.c b/drivers/reset/reset-zx2967.c
new file mode 100644
index 0000000..bc95261
--- /dev/null
+++ b/drivers/reset/reset-zx2967.c
@@ -0,0 +1,125 @@
+/*
+ * ZTE's zx2967 family reset controller driver
+ *
+ * Copyright (C) 2017 ZTE Ltd.
+ *
+ * Author: Baoyou Xie <baoyou.xie@linaro.org>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ */
+
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+
+struct zx2967_reset {
+ void __iomem *reg_base;
+ spinlock_t lock;
+ struct reset_controller_dev rcdev;
+};
+
+static int zx2967_reset_act(struct reset_controller_dev *rcdev,
+ unsigned long id, bool assert)
+{
+ struct zx2967_reset *reset = NULL;
+ u32 bank = id / 32;
+ u32 offset = id % 32;
+ u32 reg;
+ unsigned long flags;
+
+ reset = container_of(rcdev, struct zx2967_reset, rcdev);
+
+ spin_lock_irqsave(&reset->lock, flags);
+
+ reg = readl(reset->reg_base + (bank * 4));
+ if (assert)
+ reg &= ~BIT(offset);
+ else
+ reg |= BIT(offset);
+ writel(reg, reset->reg_base + (bank * 4));
+
+ spin_unlock_irqrestore(&reset->lock, flags);
+
+ return 0;
+}
+
+static int zx2967_reset_assert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ return zx2967_reset_act(rcdev, id, true);
+}
+
+static int zx2967_reset_deassert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ return zx2967_reset_act(rcdev, id, false);
+}
+
+static struct reset_control_ops zx2967_reset_ops = {
+ .assert = zx2967_reset_assert,
+ .deassert = zx2967_reset_deassert,
+};
+
+static int zx2967_reset_probe(struct platform_device *pdev)
+{
+ struct zx2967_reset *reset;
+ struct resource *res;
+
+ reset = devm_kzalloc(&pdev->dev, sizeof(*reset), GFP_KERNEL);
+ if (!reset)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ reset->reg_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(reset->reg_base))
+ return PTR_ERR(reset->reg_base);
+
+ spin_lock_init(&reset->lock);
+
+ reset->rcdev.owner = THIS_MODULE;
+ reset->rcdev.nr_resets = resource_size(res) * 8;
+ reset->rcdev.ops = &zx2967_reset_ops;
+ reset->rcdev.of_node = pdev->dev.of_node;
+
+ dev_info(&pdev->dev, "reset controller cnt:%d",
+ reset->rcdev.nr_resets);
+
+ return devm_reset_controller_register(&pdev->dev, &reset->rcdev);
+}
+
+static int zx2967_reset_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static const struct of_device_id zx2967_reset_dt_ids[] = {
+ { .compatible = "zte,zx296718-reset", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, zx2967_reset_dt_ids);
+
+static struct platform_driver zx2967_reset_driver = {
+ .probe = zx2967_reset_probe,
+ .remove = zx2967_reset_remove,
+ .driver = {
+ .name = "zx2967-reset",
+ .of_match_table = zx2967_reset_dt_ids,
+ },
+};
+
+static int __init zx2967_reset_init(void)
+{
+ return platform_driver_register(&zx2967_reset_driver);
+}
+arch_initcall(zx2967_reset_init);
+
+static void __exit zx2967_reset_exit(void)
+{
+ platform_driver_unregister(&zx2967_reset_driver);
+}
+module_exit(zx2967_reset_exit);
+
+MODULE_AUTHOR("Baoyou Xie <baoyou.xie@linaro.org>");
+MODULE_DESCRIPTION("ZTE zx2967 Reset Controller Driver");
+MODULE_LICENSE("GPL");
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 2/3] can: ti_hecc: Add TI HECC DT binding documentation
From: Yegor Yefremov @ 2017-01-16 10:59 UTC (permalink / raw)
To: Rob Herring
Cc: linux-can@vger.kernel.org, linux-omap@vger.kernel.org, devicetree,
Andrey Skvortsov, hs, Anton Glukhov, Marc Kleine-Budde,
Grim, Dennis
In-Reply-To: <20170113195633.7igh7fdlcsw5ov6g@rob-hp-laptop>
On Fri, Jan 13, 2017 at 8:56 PM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Jan 11, 2017 at 03:05:20PM +0100, yegorslists@googlemail.com wrote:
>> From: Anton Glukhov <anton.a.glukhov@gmail.com>
>>
>> DT binding documentation for TI High End CAN Controller
>>
>> Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>> ---
>> Changes v1 -> v2:
>> change compatible to "ti,am3505"
>>
>> .../devicetree/bindings/net/can/ti_hecc.txt | 31 ++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/can/ti_hecc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/can/ti_hecc.txt b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
>> new file mode 100644
>> index 0000000..ce015cf
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
>> @@ -0,0 +1,31 @@
>> +* Texas Instruments High End CAN Controller (HECC)
>> +
>> +This file provides information, what the device node
>> +for the hecc interface contains.
>> +
>> +Required properties:
>> +- compatible: "ti,am3505"
>> +- reg: offset and length of the register set for the device
>> +- interrupts: interrupt mapping for the hecc interrupts sources
>> +- clocks: clock phandles (see clock bindings for details)
>
>> +- ti,scc-ram-offset: offset to scc module ram
>> +- ti,hecc-ram-offset: offset to hecc module ram
>> +- ti,mbx-offset: offset to mailbox ram
>
> Is there not a common case that would be the default?
So far HECC is only implemented in two SoCs am3505/am3517. Both are
identical in this regard and differ in having/not having a 3D graphics
engine. So perhaps it makes sense just to move these offsets to the
driver itself?
>> +
>> +Optional properties:
>> +- ti,int-line: interrupt line
Marc suggested to convert this option to bool. Though it should be
also renamed then. HECC has basically two interrupts HECC0 and HECC1.
The one is for mailbox interrupts the other for system interrupts. But
one can map all interrupts to one "pin". This is also made in the
driver. The user can decide which one to use for all interrupts.
I'd suggest following name:
ti,use-hecc1int: if provided configures HECC to produce all interrupts
on HECC1INT interrupt line. By default HECC0INT interrupt line will be
used.
Yegor
> Needs a better description. What are valid values? This is some internal
> setting about which pin to route the interrupt output to I'm guessing.
>
>> +
>> +Example:
>> +
>> +For am3517evm board:
>> + hecc: can@0x5c050000 {
>> + compatible = "ti,am3505";
>> + status = "disabled";
>> + reg = <0x5c050000 0x4000>;
>> + interrupts = <24>;
>> + clocks = <&hecc_ck>;
>> + ti,scc-ram-offset = <0x3000>;
>> + ti,hecc-ram-offset = <0x3000>;
>> + ti,mbx-offset = <0x2000>;
>> + ti,int-line = <0>;
>> + };
>> --
>> 2.1.4
>>
^ permalink raw reply
* Re: [PATCH V2 2/5] phy: phy-exynos-pcie: Add support for Exynos PCIe phy
From: Jaehoon Chung @ 2017-01-16 11:00 UTC (permalink / raw)
To: Kishon Vijay Abraham I, linux-pci
Cc: devicetree, linux-kernel, linux-samsung-soc, bhelgaas, robh+dt,
mark.rutland, kgene, krzk, jingoohan1, vivek.gautam, pankaj.dubey,
alim.akhtar, cpgs
In-Reply-To: <587C8633.5000100@ti.com>
Hi,
On 01/16/2017 05:37 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 04 January 2017 06:04 PM, Jaehoon Chung wrote:
>> This patch supports to use Generic Phy framework for Exynos PCIe phy.
>> When Exynos that supported the pcie want to use the PCIe,
>> it needs to control the phy resgister.
>> But it should be more complex to control in their own PCIe device drivers.
>>
>> Currently, there is an exynos5440 case to support the pcie.
>> So this driver is based on Exynos5440 PCIe.
>> In future, will support the Other exynos SoCs likes exynos5433, exynos7.
>
> please re-write the commit message.
Will update the commit-message
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>> Changelog on V2:
>> - Not include the codes relevant to pci-exynos.
>> - Remove the getting child node.
>>
>> drivers/phy/Kconfig | 9 ++
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-exynos-pcie.c | 280 ++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 290 insertions(+)
>> create mode 100644 drivers/phy/phy-exynos-pcie.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index e8eb7f2..2dddef4 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -331,6 +331,15 @@ config PHY_EXYNOS5_USBDRD
>> This driver provides PHY interface for USB 3.0 DRD controller
>> present on Exynos5 SoC series.
>>
>> +config PHY_EXYNOS_PCIE
>> + bool "Exynos PCIe PHY driver"
>> + depends on ARCH_EXYNOS && OF
>
> include COMPILE_TEST
Ok.
>> + depends on PCI_EXYNOS
>
> PCI_EXYNOS should depend on PHY_EXYNOS_PCIE if at all required. Or else do away
> with this dependency.
Ok.
>> + select GENERIC_PHY
>> + help
>> + Enable PCIe PHY support for Exynos SoC series.
>> + This driver provides PHY interface for Exynos PCIe controller.
>> +
>> config PHY_PISTACHIO_USB
>> tristate "IMG Pistachio USB2.0 PHY driver"
>> depends on MACH_PISTACHIO
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 65eb2f4..081aeb4 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -37,6 +37,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o
>> phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o
>> phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o
>> obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o
>> +obj-$(CONFIG_PHY_EXYNOS_PCIE) += phy-exynos-pcie.o
>> obj-$(CONFIG_PHY_QCOM_APQ8064_SATA) += phy-qcom-apq8064-sata.o
>> obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o
>> obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2) += phy-rockchip-inno-usb2.o
>> diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c
>> new file mode 100644
>> index 0000000..b57f49b
>> --- /dev/null
>> +++ b/drivers/phy/phy-exynos-pcie.c
>> @@ -0,0 +1,280 @@
>> +/*
>> + * Samsung EXYNOS SoC series PCIe PHY driver
>> + *
>> + * Phy provider for PCIe controller on Exynos SoC series
>> + *
>> + * Copyright (C) 2016 Samsung Electronics Co., Ltd.
>
> 2017?
When i had posted the first version, it was 2016.. :)
>> + * Jaehoon Chung <jh80.chung@samsung.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/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/regmap.h>
>> +
>> +/* PCIe Purple registers */
>> +#define PCIE_PHY_GLOBAL_RESET 0x000
>> +#define PCIE_PHY_COMMON_RESET 0x004
>> +#define PCIE_PHY_CMN_REG 0x008
>> +#define PCIE_PHY_MAC_RESET 0x00c
>> +#define PCIE_PHY_PLL_LOCKED 0x010
>> +#define PCIE_PHY_TRSVREG_RESET 0x020
>> +#define PCIE_PHY_TRSV_RESET 0x024
>
> Please use BIT() macro for bit definitions.
Ok.
>> +
>> +/* PCIe PHY registers */
>> +#define PCIE_PHY_IMPEDANCE 0x004
>> +#define PCIE_PHY_PLL_DIV_0 0x008
>> +#define PCIE_PHY_PLL_BIAS 0x00c
>> +#define PCIE_PHY_DCC_FEEDBACK 0x014
>> +#define PCIE_PHY_PLL_DIV_1 0x05c
>> +#define PCIE_PHY_COMMON_POWER 0x064
>> +#define PCIE_PHY_COMMON_PD_CMN BIT(3)
>> +#define PCIE_PHY_TRSV0_EMP_LVL 0x084
>> +#define PCIE_PHY_TRSV0_DRV_LVL 0x088
>> +#define PCIE_PHY_TRSV0_RXCDR 0x0ac
>> +#define PCIE_PHY_TRSV0_POWER 0x0c4
>> +#define PCIE_PHY_TRSV0_PD_TSV BIT(7)
>> +#define PCIE_PHY_TRSV0_LVCC 0x0dc
>> +#define PCIE_PHY_TRSV1_EMP_LVL 0x144
>> +#define PCIE_PHY_TRSV1_RXCDR 0x16c
>> +#define PCIE_PHY_TRSV1_POWER 0x184
>> +#define PCIE_PHY_TRSV1_PD_TSV BIT(7)
>> +#define PCIE_PHY_TRSV1_LVCC 0x19c
>> +#define PCIE_PHY_TRSV2_EMP_LVL 0x204
>> +#define PCIE_PHY_TRSV2_RXCDR 0x22c
>> +#define PCIE_PHY_TRSV2_POWER 0x244
>> +#define PCIE_PHY_TRSV2_PD_TSV BIT(7)
>> +#define PCIE_PHY_TRSV2_LVCC 0x25c
>> +#define PCIE_PHY_TRSV3_EMP_LVL 0x2c4
>> +#define PCIE_PHY_TRSV3_RXCDR 0x2ec
>> +#define PCIE_PHY_TRSV3_POWER 0x304
>> +#define PCIE_PHY_TRSV3_PD_TSV BIT(7)
>> +#define PCIE_PHY_TRSV3_LVCC 0x31c
>> +
>> +struct exynos_pcie_phy_data {
>> + struct phy_ops *ops;
>> +};
>> +
>> +/* For Exynos pcie phy */
>> +struct exynos_pcie_phy {
>> + const struct exynos_pcie_phy_data *drv_data;
>> + void __iomem *phy_base;
>> + void __iomem *blk_base; /* For exynos5440 */
>> +};
>> +
>> +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset)
>> +{
>> + writel(val, base + offset);
>> +}
>> +
>> +static u32 exynos_pcie_phy_readl(void __iomem *base, u32 offset)
>> +{
>> + return readl(base + offset);
>> +}
>> +
>> +/* For Exynos5440 specific functions */
>> +static int exynos5440_pcie_phy_init(struct phy *phy)
>> +{
>> + struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +
>> + /* DCC feedback control off */
>> + exynos_pcie_phy_writel(ep->phy_base, 0x29, PCIE_PHY_DCC_FEEDBACK);
>> +
>> + /* set TX/RX impedance */
>> + exynos_pcie_phy_writel(ep->phy_base, 0xd5, PCIE_PHY_IMPEDANCE);
>> +
>> + /* set 50Mhz PHY clock */
>> + exynos_pcie_phy_writel(ep->phy_base, 0x14, PCIE_PHY_PLL_DIV_0);
>> + exynos_pcie_phy_writel(ep->phy_base, 0x12, PCIE_PHY_PLL_DIV_1);
>> +
>> + /* set TX Differential output for lane 0 */
>> + exynos_pcie_phy_writel(ep->phy_base, 0x7f, PCIE_PHY_TRSV0_DRV_LVL);
>> +
>> + /* set TX Pre-emphasis Level Control for lane 0 to minimum */
>> + exynos_pcie_phy_writel(ep->phy_base, 0x0, PCIE_PHY_TRSV0_EMP_LVL);
>> +
>> + /* set RX clock and data recovery bandwidth */
>> + exynos_pcie_phy_writel(ep->phy_base, 0xe7, PCIE_PHY_PLL_BIAS);
>> + exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV0_RXCDR);
>> + exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV1_RXCDR);
>> + exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV2_RXCDR);
>> + exynos_pcie_phy_writel(ep->phy_base, 0x82, PCIE_PHY_TRSV3_RXCDR);
>> +
>> + /* change TX Pre-emphasis Level Control for lanes */
>> + exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV0_EMP_LVL);
>> + exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV1_EMP_LVL);
>> + exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV2_EMP_LVL);
>> + exynos_pcie_phy_writel(ep->phy_base, 0x39, PCIE_PHY_TRSV3_EMP_LVL);
>> +
>> + /* set LVCC */
>> + exynos_pcie_phy_writel(ep->phy_base, 0x20, PCIE_PHY_TRSV0_LVCC);
>> + exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV1_LVCC);
>> + exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV2_LVCC);
>> + exynos_pcie_phy_writel(ep->phy_base, 0xa0, PCIE_PHY_TRSV3_LVCC);
>
> I'm starting to dis-like all this hard-coded hw params. All this should come
> from dt. Define a dt binding like this for all hw params..
> phy,tx-differential = <val, reg-offset, mask>
>
> and have one API in phy-core to do all these settings.
Will check.
>> +
>> + /* pulse for common reset */
>> + exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_COMMON_RESET);
>> + udelay(500);
>
> how did you get this delay value? Adding a comment might help.
Well..Actually i don't know why udelay(500) was defined.
When i had started the refactoring pci-exynos, there was one problem..
I didn't have Exynos5440 TRM and information..i can't check anything for exyno5440.
(I don't know who is using EXYNOS5440 pci.)
the entire codes are just moved from pci-exynos.c.
When i want to upstream exynos5433 pcie, first step is the cleaning pci-exynos.c relevant to exynos5440.
It was too complex to support the other Exynos SoCs.
Otherwise, it needs to add the new file for exynos5433.
>> + exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
>> +
>> + return 0;
>> +}
>> +
>> +static int exynos5440_pcie_phy_power_on(struct phy *phy)
>> +{
>> + struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> + u32 val;
>> +
>> + exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_COMMON_RESET);
>> + exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_CMN_REG);
>> + exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSVREG_RESET);
>> + exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_TRSV_RESET);
>> +
>> + val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
>> + val &= ~PCIE_PHY_COMMON_PD_CMN;
>> + exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
>> +
>> + val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
>> + val &= ~PCIE_PHY_TRSV0_PD_TSV;
>> + exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
>> +
>> + val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
>> + val &= ~PCIE_PHY_TRSV1_PD_TSV;
>> + exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
>> +
>> + val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
>> + val &= ~PCIE_PHY_TRSV2_PD_TSV;
>> + exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
>> +
>> + val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
>> + val &= ~PCIE_PHY_TRSV3_PD_TSV;
>> + exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
>> +
>> + return 0;
>> +}
>> +
>> +static int exynos5440_pcie_phy_power_off(struct phy *phy)
>> +{
>> + struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> + u32 val;
>> +
>> + while (exynos_pcie_phy_readl(ep->phy_base,
>> + PCIE_PHY_PLL_LOCKED) == 0) {
>> + val = exynos_pcie_phy_readl(ep->blk_base,
>> + PCIE_PHY_PLL_LOCKED);
>> + dev_info(&phy->dev, "PLL Locked: 0x%x\n", val);
>> + }
>> +
>> + val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_COMMON_POWER);
>> + val |= PCIE_PHY_COMMON_PD_CMN;
>> + exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_COMMON_POWER);
>> +
>> + val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV0_POWER);
>> + val |= PCIE_PHY_TRSV0_PD_TSV;
>> + exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV0_POWER);
>> +
>> + val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV1_POWER);
>> + val |= PCIE_PHY_TRSV1_PD_TSV;
>> + exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV1_POWER);
>> +
>> + val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV2_POWER);
>> + val |= PCIE_PHY_TRSV2_PD_TSV;
>> + exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV2_POWER);
>> +
>> + val = exynos_pcie_phy_readl(ep->phy_base, PCIE_PHY_TRSV3_POWER);
>> + val |= PCIE_PHY_TRSV3_PD_TSV;
>> + exynos_pcie_phy_writel(ep->phy_base, val, PCIE_PHY_TRSV3_POWER);
>> +
>> + return 0;
>> +}
>> +
>> +static int exynos5440_pcie_phy_reset(struct phy *phy)
>> +{
>> + struct exynos_pcie_phy *ep = phy_get_drvdata(phy);
>> +
>> + exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_MAC_RESET);
>> + exynos_pcie_phy_writel(ep->blk_base, 1, PCIE_PHY_GLOBAL_RESET);
>> + exynos_pcie_phy_writel(ep->blk_base, 0, PCIE_PHY_GLOBAL_RESET);
>> +
>> + return 0;
>> +}
>> +
>> +static struct phy_ops exynos5440_phy_ops = {
>> + .init = exynos5440_pcie_phy_init,
>> + .power_on = exynos5440_pcie_phy_power_on,
>> + .power_off = exynos5440_pcie_phy_power_off,
>> + .reset = exynos5440_pcie_phy_reset,
>
> add .owner
Will fix.
>> +};
>> +
>> +static const struct exynos_pcie_phy_data exynos5440_pcie_phy_data = {
>> + .ops = &exynos5440_phy_ops,
>
> why do you need a wrapper for phy_ops?
My main goal is the upstreaming exynos5433 pci, not exynos5440.
There are many different with exynos5440 and exynos5433.
As i know, only exynos5440 has the big different with other Exynos variants.
I think i can be removed in this patch. When i upstream the other SoCs, I will use the wrapper.
Best Regards,
Jaehoon Chung
>> +};
>> +
>> +static const struct of_device_id exynos_pcie_phy_match[] = {
>> + {
>> + .compatible = "samsung,exynos5440-pcie-phy",
>> + .data = &exynos5440_pcie_phy_data,
>> + },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, exynos_pcie_phy_match);
>> +
>> +static int exynos_pcie_phy_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct exynos_pcie_phy *exynos_phy;
>> + struct phy *generic_phy;
>> + struct phy_provider *phy_provider;
>> + struct resource *res;
>> + const struct exynos_pcie_phy_data *drv_data;
>> +
>> + drv_data = of_device_get_match_data(dev);
>> + if (!drv_data)
>> + return -ENODEV;
>> +
>> + exynos_phy = devm_kzalloc(dev, sizeof(*exynos_phy), GFP_KERNEL);
>> + if (!exynos_phy)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + exynos_phy->phy_base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(exynos_phy->phy_base))
>> + return PTR_ERR(exynos_phy->phy_base);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> + exynos_phy->blk_base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(exynos_phy->phy_base))
>> + return PTR_ERR(exynos_phy->phy_base);
>> +
>> + exynos_phy->drv_data = drv_data;
>> +
>> + generic_phy = devm_phy_create(dev, dev->of_node, drv_data->ops);
>> + if (IS_ERR(generic_phy)) {
>> + dev_err(dev, "failed to create PHY\n");
>> + return PTR_ERR(generic_phy);
>> + }
>> +
>> + phy_set_drvdata(generic_phy, exynos_phy);
>> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> +
>> + return PTR_ERR_OR_ZERO(phy_provider);
>> +}
>> +
>> +static struct platform_driver exynos_pcie_phy_driver = {
>> + .probe = exynos_pcie_phy_probe,
>> + .driver = {
>> + .of_match_table = exynos_pcie_phy_match,
>> + .name = "exynos_pcie_phy",
>> + }
>> +};
>> +module_platform_driver(exynos_pcie_phy_driver);
>>
> Thanks
> Kishon
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply
* Re: [PATCH v4 2/2] mtd: spi-nor: add rockchip serial flash controller driver
From: Cyrille Pitchen @ 2017-01-16 11:10 UTC (permalink / raw)
To: Shawn Lin, David Woodhouse, Brian Norris
Cc: Marek Vasut, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Heiko Stuebner
In-Reply-To: <1481794068-241619-3-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Hi Shawn,
Le 15/12/2016 à 10:27, Shawn Lin a écrit :
> Add rockchip serial flash controller driver
>
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>
> ---
>
> Changes in v4:
> - simplify the code of get_if_type
> - use dma_dir to simplify the code
> - simplify the rockchip_sfc_do_rd_wr
> - some minor improvements
> - add reset controller when doing resume
>
> Changes in v3:
> - use io{read32,write32}_rep to simplify the corner cases
> - remove more unnecessary bit definitions
> - some minor comment fixes and improvement
> - fix wrong unregister function
> - unify more code
> - use nor to avoid constantly replicating the whole
> sfc->flash[sfc->num_chip].nor
> - add email for MODULE_AUTHOR
> - remove #if 1 --- #endif
> - extract DMA code to imporve the code structure
> - reset all when failing to do dma
> - pass sfc to get_if_type
> - rename sfc-no-dma to sfc-no-DMA
>
> Changes in v2:
> - fix typos
> - add some comment for buffer and others operations
> - rename SFC_MAX_CHIP_NUM to MAX_CHIPSELECT_NUM
> - use u8 for cs
> - return -EINVAL for default case of get_if_type
> - use readl_poll_*() to check timeout cases
> - simplify and clarify some condition checks
> - rework the bitshifts to simplify the code
> - define SFC_CMD_DUMMY(x)
> - fix ummap for dma read path and finish all the
> cache maintenance.
> - rename to rockchip_sfc_chip_priv and embed struct spi_nor
> in it.
> - add MODULE_AUTHOR
> - add runtime PM and general PM support.
> - Thanks for Marek's comments. Link:
> http://lists.infradead.org/pipermail/linux-mtd/2016-November/070321.html
>
> MAINTAINERS | 8 +
> drivers/mtd/spi-nor/Kconfig | 7 +
> drivers/mtd/spi-nor/Makefile | 1 +
> drivers/mtd/spi-nor/rockchip-sfc.c | 872 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 888 insertions(+)
> create mode 100644 drivers/mtd/spi-nor/rockchip-sfc.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1cd38a7..eb7e06d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10266,6 +10266,14 @@ L: linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> S: Odd Fixes
> F: drivers/tty/serial/rp2.*
>
> +ROCKCHIP SERIAL FLASH CONTROLLER DRIVER
> +M: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> +L: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> +L: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> +S: Maintained
> +F: Documentation/devicetree/bindings/mtd/rockchip-sfc.txt
> +F: drivers/mtd/spi-nor/rockchip-sfc.c
> +
> ROSE NETWORK LAYER
> M: Ralf Baechle <ralf-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org>
> L: linux-hams-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 4a682ee..bf783a8 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -76,4 +76,11 @@ config SPI_NXP_SPIFI
> Flash. Enable this option if you have a device with a SPIFI
> controller and want to access the Flash as a mtd device.
>
> +config SPI_ROCKCHIP_SFC
> + tristate "Rockchip Serial Flash Controller(SFC)"
> + depends on ARCH_ROCKCHIP || COMPILE_TEST
> + depends on HAS_IOMEM && HAS_DMA
> + help
> + This enables support for rockchip serial flash controller.
> +
> endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index 121695e..364d4c6 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -5,3 +5,4 @@ obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
> obj-$(CONFIG_SPI_HISI_SFC) += hisi-sfc.o
> obj-$(CONFIG_MTD_MT81xx_NOR) += mtk-quadspi.o
> obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o
> +obj-$(CONFIG_SPI_ROCKCHIP_SFC) += rockchip-sfc.o
> diff --git a/drivers/mtd/spi-nor/rockchip-sfc.c b/drivers/mtd/spi-nor/rockchip-sfc.c
> new file mode 100644
> index 0000000..102c08f
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/rockchip-sfc.c
> @@ -0,0 +1,872 @@
> +/*
> + * Rockchip Serial Flash Controller Driver
> + *
> + * Copyright (c) 2016, Rockchip Inc.
> + * Author: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/spi-nor.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +
> +/* System control */
> +#define SFC_CTRL 0x0
> +#define SFC_CTRL_COMMON_BITS_1 0x0
> +#define SFC_CTRL_COMMON_BITS_2 0x1
> +#define SFC_CTRL_COMMON_BITS_4 0x2
> +#define SFC_CTRL_DATA_BITS_SHIFT 12
> +#define SFC_CTRL_ADDR_BITS_SHIFT 10
> +#define SFC_CTRL_CMD_BITS_SHIFT 8
> +#define SFC_CTRL_PHASE_SEL_NEGETIVE BIT(1)
> +
> +/* Interrupt mask */
> +#define SFC_IMR 0x4
> +#define SFC_IMR_RX_FULL BIT(0)
> +#define SFC_IMR_RX_UFLOW BIT(1)
> +#define SFC_IMR_TX_OFLOW BIT(2)
> +#define SFC_IMR_TX_EMPTY BIT(3)
> +#define SFC_IMR_TRAN_FINISH BIT(4)
> +#define SFC_IMR_BUS_ERR BIT(5)
> +#define SFC_IMR_NSPI_ERR BIT(6)
> +#define SFC_IMR_DMA BIT(7)
> +/* Interrupt clear */
> +#define SFC_ICLR 0x8
> +#define SFC_ICLR_RX_FULL BIT(0)
> +#define SFC_ICLR_RX_UFLOW BIT(1)
> +#define SFC_ICLR_TX_OFLOW BIT(2)
> +#define SFC_ICLR_TX_EMPTY BIT(3)
> +#define SFC_ICLR_TRAN_FINISH BIT(4)
> +#define SFC_ICLR_BUS_ERR BIT(5)
> +#define SFC_ICLR_NSPI_ERR BIT(6)
> +#define SFC_ICLR_DMA BIT(7)
> +/* FIFO threshold level */
> +#define SFC_FTLR 0xc
> +#define SFC_FTLR_TX_SHIFT 0
> +#define SFC_FTLR_TX_MASK 0x1f
> +#define SFC_FTLR_RX_SHIFT 8
> +#define SFC_FTLR_RX_MASK 0x1f
> +/* Reset FSM and FIFO */
> +#define SFC_RCVR 0x10
> +#define SFC_RCVR_RESET BIT(0)
> +/* Enhanced mode */
> +#define SFC_AX 0x14
> +/* Address Bit number */
> +#define SFC_ABIT 0x18
> +/* Interrupt status */
> +#define SFC_ISR 0x1c
> +#define SFC_ISR_RX_FULL_SHIFT BIT(0)
> +#define SFC_ISR_RX_UFLOW_SHIFT BIT(1)
> +#define SFC_ISR_TX_OFLOW_SHIFT BIT(2)
> +#define SFC_ISR_TX_EMPTY_SHIFT BIT(3)
> +#define SFC_ISR_TX_FINISH_SHIFT BIT(4)
> +#define SFC_ISR_BUS_ERR_SHIFT BIT(5)
> +#define SFC_ISR_NSPI_ERR_SHIFT BIT(6)
> +#define SFC_ISR_DMA_SHIFT BIT(7)
> +/* FIFO status */
> +#define SFC_FSR 0x20
> +#define SFC_FSR_TX_IS_FULL BIT(0)
> +#define SFC_FSR_TX_IS_EMPTY BIT(1)
> +#define SFC_FSR_RX_IS_EMPTY BIT(2)
> +#define SFC_FSR_RX_IS_FULL BIT(3)
> +/* FSM status */
> +#define SFC_SR 0x24
> +#define SFC_SR_IS_IDLE 0x0
> +#define SFC_SR_IS_BUSY 0x1
> +/* Raw interrupt status */
> +#define SFC_RISR 0x28
> +#define SFC_RISR_RX_FULL BIT(0)
> +#define SFC_RISR_RX_UNDERFLOW BIT(1)
> +#define SFC_RISR_TX_OVERFLOW BIT(2)
> +#define SFC_RISR_TX_EMPTY BIT(3)
> +#define SFC_RISR_TRAN_FINISH BIT(4)
> +#define SFC_RISR_BUS_ERR BIT(5)
> +#define SFC_RISR_NSPI_ERR BIT(6)
> +#define SFC_RISR_DMA BIT(7)
> +/* Master trigger */
> +#define SFC_DMA_TRIGGER 0x80
> +/* Src or Dst addr for master */
> +#define SFC_DMA_ADDR 0x84
> +/* Command */
> +#define SFC_CMD 0x100
> +#define SFC_CMD_IDX_SHIFT 0
> +#define SFC_CMD_DUMMY_SHIFT 8
> +#define SFC_CMD_DIR_RD 0
> +#define SFC_CMD_DIR_WR 1
> +#define SFC_CMD_DIR_SHIFT 12
> +#define SFC_CMD_ADDR_ZERO (0x0 << 14)
> +#define SFC_CMD_ADDR_24BITS (0x1 << 14)
> +#define SFC_CMD_ADDR_32BITS (0x2 << 14)
> +#define SFC_CMD_ADDR_FRS (0x3 << 14)
> +#define SFC_CMD_TRAN_BYTES_SHIFT 16
> +#define SFC_CMD_CS_SHIFT 30
> +/* Address */
> +#define SFC_ADDR 0x104
> +/* Data */
> +#define SFC_DATA 0x108
> +
> +#define SFC_MAX_CHIPSELECT_NUM 4
> +#define SFC_DMA_MAX_LEN 0x4000
> +#define SFC_CMD_DUMMY(x) \
> + ((x) << SFC_CMD_DUMMY_SHIFT)
> +
> +enum rockchip_sfc_iftype {
> + IF_TYPE_STD,
> + IF_TYPE_DUAL,
> + IF_TYPE_QUAD,
> +};
> +
> +struct rockchip_sfc;
> +struct rockchip_sfc_chip_priv {
> + u8 cs;
> + u32 clk_rate;
> + struct spi_nor nor;
> + struct rockchip_sfc *sfc;
> +};
> +
> +struct rockchip_sfc {
> + struct device *dev;
> + struct mutex lock;
> + void __iomem *regbase;
> + struct clk *hclk;
> + struct clk *clk;
> + /* virtual mapped addr for dma_buffer */
> + void *buffer;
> + dma_addr_t dma_buffer;
> + struct completion cp;
> + struct rockchip_sfc_chip_priv flash[SFC_MAX_CHIPSELECT_NUM];
> + u32 num_chip;
> + bool use_dma;
> + /* use negative edge of hclk to latch data */
> + bool negative_edge;
> +};
> +
> +static int get_if_type(struct rockchip_sfc *sfc, enum read_mode flash_read)
> +{
> + if (flash_read == SPI_NOR_DUAL)
> + return IF_TYPE_DUAL;
> + else if (flash_read == SPI_NOR_QUAD)
> + return IF_TYPE_QUAD;
> + else if (flash_read == SPI_NOR_NORMAL ||
> + flash_read == SPI_NOR_FAST)
> + return IF_TYPE_STD;
> +
> + dev_err(sfc->dev, "unsupported SPI read mode\n");
> + return -EINVAL;
> +}
> +
> +static int rockchip_sfc_reset(struct rockchip_sfc *sfc)
> +{
> + int err;
> + u32 status;
> +
> + writel_relaxed(SFC_RCVR_RESET, sfc->regbase + SFC_RCVR);
> +
> + err = readl_poll_timeout(sfc->regbase + SFC_RCVR, status,
> + !(status & SFC_RCVR_RESET), 20,
> + jiffies_to_usecs(HZ));
> + if (err)
> + dev_err(sfc->dev, "SFC reset never finished\n");
> +
> + /* Still need to clear the masked interrupt from RISR */
> + writel_relaxed(SFC_ICLR_RX_FULL | SFC_ICLR_RX_UFLOW |
> + SFC_ICLR_TX_OFLOW | SFC_ICLR_TX_EMPTY |
> + SFC_ICLR_TRAN_FINISH | SFC_ICLR_BUS_ERR |
> + SFC_ICLR_NSPI_ERR | SFC_ICLR_DMA,
> + sfc->regbase + SFC_ICLR);
> + return err;
> +}
> +
> +static int rockchip_sfc_init(struct rockchip_sfc *sfc)
> +{
> + int err;
> +
> + err = rockchip_sfc_reset(sfc);
> + if (err)
> + return err;
> +
> + /* Mask all eight interrupts */
> + writel_relaxed(0xff, sfc->regbase + SFC_IMR);
> +
> + /*
> + * Phase configure for sfc to latch data by using
> + * ahb clock, and this configuration should be Soc
> + * specific.
> + */
> + if (sfc->negative_edge)
> + writel_relaxed(SFC_CTRL_PHASE_SEL_NEGETIVE,
> + sfc->regbase + SFC_CTRL);
> + else
> + writel_relaxed(0, sfc->regbase + SFC_CTRL);
> +
> + return 0;
> +}
> +
> +static int rockchip_sfc_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> + struct rockchip_sfc_chip_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + int ret;
> +
> + mutex_lock(&sfc->lock);
> + pm_runtime_get_sync(sfc->dev);
> +
> + ret = clk_set_rate(sfc->clk, priv->clk_rate);
> + if (ret)
> + goto out;
> +
> + ret = clk_prepare_enable(sfc->clk);
> + if (ret)
> + goto out;
> +
> + return 0;
> +
> +out:
> + mutex_unlock(&sfc->lock);
> + return ret;
> +}
> +
> +static void rockchip_sfc_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
> +{
> + struct rockchip_sfc_chip_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> +
> + clk_disable_unprepare(sfc->clk);
> + mutex_unlock(&sfc->lock);
> + pm_runtime_mark_last_busy(sfc->dev);
> + pm_runtime_put_autosuspend(sfc->dev);
> +}
> +
> +static int rockchip_sfc_wait_op_finish(struct rockchip_sfc *sfc)
> +{
> + int err;
> + u32 status;
> +
> + /*
> + * Note: tx and rx share the same fifo, so the rx's water level
> + * is the same as rx's, which means this function could be reused
> + * for checking the read operations as well.
> + */
> + err = readl_poll_timeout(sfc->regbase + SFC_FSR, status,
> + status & SFC_FSR_TX_IS_EMPTY,
> + 20, jiffies_to_usecs(2 * HZ));
> + if (err)
> + dev_err(sfc->dev, "SFC fifo never empty\n");
> +
> + return err;
> +}
> +
> +static int rockchip_sfc_op_reg(struct spi_nor *nor,
> + u8 opcode, int len, u8 optype)
> +{
> + struct rockchip_sfc_chip_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + u32 reg;
> + bool tx_no_empty, rx_no_empty, is_busy;
> + int err;
> +
> + reg = readl_relaxed(sfc->regbase + SFC_FSR);
> + tx_no_empty = !(reg & SFC_FSR_TX_IS_EMPTY);
> + rx_no_empty = !(reg & SFC_FSR_RX_IS_EMPTY);
> +
> + is_busy = readl_relaxed(sfc->regbase + SFC_SR);
> +
> + if (tx_no_empty || rx_no_empty || is_busy) {
> + err = rockchip_sfc_reset(sfc);
> + if (err)
> + return err;
> + }
> +
> + reg = opcode << SFC_CMD_IDX_SHIFT;
> + reg |= len << SFC_CMD_TRAN_BYTES_SHIFT;
> + reg |= priv->cs << SFC_CMD_CS_SHIFT;
> + reg |= optype << SFC_CMD_DIR_SHIFT;
> +
> + writel_relaxed(reg, sfc->regbase + SFC_CMD);
> +
> + return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static void rockchip_sfc_read_fifo(struct rockchip_sfc *sfc, u8 *buf, int len)
> +{
> + u32 tmp, i;
> + int total_len = len;
> +
> + /* 32-bit access only */
> + if (len >= 4 && !((u32)buf & 0x03)) {
> + ioread32_rep(sfc->regbase + SFC_DATA, buf, len >> 2);
> + len %= 4;
> + buf += total_len - len;
> + }
> +
> + /* read the rest bytes */
> + for (i = 0; i < len; i++) {
> + if (!(i & 0x03))
> + tmp = readl_relaxed(sfc->regbase + SFC_DATA);
> + buf[i] = (tmp >> ((i & 0x03) * 8)) & 0xff;
> + }
> +}
> +
> +static int rockchip_sfc_read_reg(struct spi_nor *nor, u8 opcode,
> + u8 *buf, int len)
> +{
> + struct rockchip_sfc_chip_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + int ret;
> +
> + ret = rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_RD);
> + if (ret)
> + return ret;
> +
> + rockchip_sfc_read_fifo(sfc, buf, len);
> +
> + return 0;
> +}
> +
> +static int rockchip_sfc_write_reg(struct spi_nor *nor, u8 opcode,
> + u8 *buf, int len)
> +{
> + struct rockchip_sfc_chip_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + u32 dwords;
> +
> + /* Align bytes to dwords */
> + dwords = DIV_ROUND_UP(len, sizeof(u32));
> + iowrite32_rep(sfc->regbase + SFC_DATA, buf, dwords);
> +
> + return rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_WR);
> +}
> +
> +static inline void rockchip_sfc_setup_transfer(struct spi_nor *nor,
> + loff_t from_to,
> + size_t len, u8 op_type)
> +{
> + struct rockchip_sfc_chip_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + u32 reg;
> + u8 if_type = 0;
> +
> + if_type = get_if_type(sfc, nor->flash_read);
> + writel_relaxed((if_type << SFC_CTRL_DATA_BITS_SHIFT) |
> + (if_type << SFC_CTRL_ADDR_BITS_SHIFT) |
> + (if_type << SFC_CTRL_CMD_BITS_SHIFT) |
> + (sfc->negative_edge ? SFC_CTRL_PHASE_SEL_NEGETIVE : 0),
> + sfc->regbase + SFC_CTRL);
Marek rose an issue when commenting v3:
Looking at that code it seems that even if the hardware can support SPI
1-1-n protocols, this driver actually allows SPI n-n-n protocols.
However with the current spi-nor framework, the values of the enum
read_mode must be understood this way:
SPI_NOR_FAST or SPI_NOR_NORMAL: SPI 1-1-1 protocol
SPI_NOR_DUAL: SPI 1-1-2 protocol
SPI_NOR_QUAD: SPI 1-1-4 protocol
Support to other SPI protocols such as SPI 1-4-4 or SPI 4-4-4 as not been
accepted in mainline yet.
Below in this driver, spi_nor_scan() is called with the value SPI_NOR_QUAD
hence it asks the spi-nor framework for using the SPI 1-1-4 (and not SPI
4-4-4) *when supported by the QSPI memory*.
Also since the driver was tested with a Winbond w25q256 memory, let me warn
you that currently the SPI_NOR_QUAD_READ flag is *NOT* set in the "w25q256"
entry of the spi_nor_ids[] array. So this claims this memory is not capable
of using the SPI 1-1-4 protocol even if the Winbond memory actually
supports this protocol. Then the spi-nor framework selects the SPI 1-1-1
protocol as a fallback.
That's why you have succeeded in using your driver but it would have failed
with another QSPI memory with its SPI_NOR_QUAD_READ flag due to a protocol
mismatch.
So with the current spi-nor framework you must set
SFC_CTRL_{DATA|ADDR}_BITS_SHIFT to IF_TYPE_STD.
Later, once the patch adding support to other SPI protocols would have been
accepted in mainline, you could update your driver to tell the spi-nor
framework that the rockchip controller also supports SPI 1-2-2 and SPI
1-4-4 protocols.
> +
> + if (op_type == SFC_CMD_DIR_WR)
> + reg = nor->program_opcode << SFC_CMD_IDX_SHIFT;
> + else
> + reg = nor->read_opcode << SFC_CMD_IDX_SHIFT;
> +
> + reg |= op_type << SFC_CMD_DIR_SHIFT;
> + reg |= (nor->addr_width == 4) ?
> + SFC_CMD_ADDR_32BITS : SFC_CMD_ADDR_24BITS;
> +
> + reg |= priv->cs << SFC_CMD_CS_SHIFT;
> + reg |= len << SFC_CMD_TRAN_BYTES_SHIFT;
Looking at the definitions of SFC_CMD_TRAN_BYTES_SHIFT (16) and
SFC_CMD_CS_SHIFT (30), I understand that the bitfield for the transfer
length lays between bits 16 and 30 hence a 14 bit value at most.
So what if len is greater than 16384? It overflows in the cs bitfield?
You should apply masks to avoid such overflows and also test the len value
then report the actual number of transferred bytes.
> +
> + if (op_type == SFC_CMD_DIR_RD)
> + reg |= SFC_CMD_DUMMY(nor->read_dummy);
> +
> + /* Should minus one as 0x0 means 1 bit flash address */
> + writel_relaxed(nor->addr_width * 8 - 1, sfc->regbase + SFC_ABIT);
> + writel_relaxed(reg, sfc->regbase + SFC_CMD);
> + writel_relaxed(from_to, sfc->regbase + SFC_ADDR);
> +}
> +
> +static int rockchip_sfc_do_dma_transfer(struct spi_nor *nor, loff_t from_to,
> + dma_addr_t dma_buf, size_t len,
> + u8 op_type)
> +{
> + struct rockchip_sfc_chip_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + u32 reg;
> + int err = 0;
> +
> + init_completion(&sfc->cp);
> +
> + writel_relaxed(SFC_ICLR_RX_FULL | SFC_ICLR_RX_UFLOW |
> + SFC_ICLR_TX_OFLOW | SFC_ICLR_TX_EMPTY |
> + SFC_ICLR_TRAN_FINISH | SFC_ICLR_BUS_ERR |
> + SFC_ICLR_NSPI_ERR | SFC_ICLR_DMA,
> + sfc->regbase + SFC_ICLR);
> +
> + /* Enable transfer complete interrupt */
> + reg = readl_relaxed(sfc->regbase + SFC_IMR);
> + reg &= ~SFC_IMR_TRAN_FINISH;
> + writel_relaxed(reg, sfc->regbase + SFC_IMR);
> +
> + rockchip_sfc_setup_transfer(nor, from_to, len, op_type);
> + writel_relaxed(dma_buf, sfc->regbase + SFC_DMA_ADDR);
> +
> + /*
> + * Start dma but note that the sfc->dma_buffer is derived from
> + * dmam_alloc_coherent so we don't actually need any sync operations
> + * for coherent dma memory.
> + */
> + writel_relaxed(0x1, sfc->regbase + SFC_DMA_TRIGGER);
> +
> + /* Wait for the interrupt. */
> + if (!wait_for_completion_timeout(&sfc->cp, msecs_to_jiffies(2000))) {
> + dev_err(sfc->dev, "DMA wait for transfer finish timeout\n");
> + err = -ETIMEDOUT;
> + }
> +
> + /* Disable transfer finish interrupt */
> + reg = readl_relaxed(sfc->regbase + SFC_IMR);
> + reg |= SFC_IMR_TRAN_FINISH;
> + writel_relaxed(reg, sfc->regbase + SFC_IMR);
> +
> + if (err) {
> + rockchip_sfc_reset(sfc);
> + return err;
> + }
> +
> + return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static inline int rockchip_sfc_pio_write(struct rockchip_sfc *sfc, u_char *buf,
> + size_t len)
> +{
> + u32 dwords;
> +
> + /*
> + * Align bytes to dwords, although we will write some extra
> + * bytes to fifo but the transfer bytes number in SFC_CMD
> + * register will make sure we just send out the expected
> + * byte numbers and the extra bytes will be clean before
> + * setting up the next transfer. We should always round up
> + * to align to DWORD as the ahb for Rockchip Socs won't
> + * support non-aligned-to-DWORD transfer.
> + */
> + dwords = DIV_ROUND_UP(len, sizeof(u32));
> + iowrite32_rep(sfc->regbase + SFC_DATA, buf, dwords);
> +
> + return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static inline int rockchip_sfc_pio_read(struct rockchip_sfc *sfc, u_char *buf,
> + size_t len)
> +{
> + rockchip_sfc_read_fifo(sfc, buf, len);
> +
> + return rockchip_sfc_wait_op_finish(sfc);
> +}
> +
> +static int rockchip_sfc_pio_transfer(struct spi_nor *nor, loff_t from_to,
> + size_t len, u_char *buf, u8 op_type)
> +{
> + struct rockchip_sfc_chip_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> +
> + rockchip_sfc_setup_transfer(nor, from_to, len, op_type);
> +
> + if (op_type == SFC_CMD_DIR_WR)
> + return rockchip_sfc_pio_write(sfc, buf, len);
> + else
> + return rockchip_sfc_pio_read(sfc, buf, len);
> +}
> +
> +static int rockchip_sfc_dma_transfer(struct spi_nor *nor, loff_t from_to,
> + size_t len, u_char *buf, u8 op_type)
> +{
> + struct rockchip_sfc_chip_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + size_t offset;
> + int ret;
> + dma_addr_t dma_addr = 0;
> + int dma_dir;
> +
> + dma_dir = (op_type == SFC_CMD_DIR_RD) ?
> + DMA_FROM_DEVICE : DMA_TO_DEVICE;
> +
> + for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) {
> + size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset);
> +
> + dma_addr = dma_map_single(NULL, (void *)buf, trans, dma_dir);
not good: buf may have been allocated with vmalloc() hence the pages of buf
are not garanteed to be contiguous in physical memory.
Just write a ubifs image into your QSPI memory and try to mount it. You are
very likely to notice some issues/crashes.
> +
> + if (dma_mapping_error(sfc->dev, dma_addr)) {
> + /*
> + * If we use pre-allocated dma_buffer, we need to
> + * do a copy here.
> + */
> + if (op_type == SFC_CMD_DIR_WR)
> + memcpy(sfc->buffer, buf + offset, trans);
> +
> + dma_addr = 0;
> + }
> +
> + if (op_type == SFC_CMD_DIR_WR)
> + /*
> + * Flush the write data from write_buf to dma_addr
> + * if using dynamic allocated dma buffer before dma
> + * moves data from dma_addr to fifo.
> + */
> + dma_sync_single_for_device(sfc->dev, dma_addr,
> + trans, DMA_TO_DEVICE);
> +
> +
> + /* If failing to map dma, use pre-allocated area instead */
> + ret = rockchip_sfc_do_dma_transfer(nor, from_to + offset,
> + dma_addr ? dma_addr :
> + sfc->dma_buffer,
> + trans, op_type);
> +
> + if (dma_addr) {
> + /*
> + * Invalidate the read data from dma_addr if using
> + * dynamic allocated dma buffer after dma moves data
> + * from fifo to dma_addr.
> + */
> + if (op_type == SFC_CMD_DIR_RD)
> + dma_sync_single_for_cpu(sfc->dev, dma_addr,
> + trans, DMA_FROM_DEVICE);
> +
> + dma_unmap_single(NULL, dma_addr,
> + trans, dma_dir);
> + }
> +
> + if (ret) {
> + dev_warn(nor->dev, "DMA read timeout\n");
> + return ret;
> + }
> + /*
> + * If we use pre-allocated dma_buffer for read, we need to
> + * do a copy here.
> + */
> + if (!dma_addr && (op_type == SFC_CMD_DIR_RD))
> + memcpy(buf + offset, sfc->buffer, trans);
> + }
> +
> + return len;
> +}
> +
> +static ssize_t rockchip_sfc_do_rd_wr(struct spi_nor *nor, loff_t from_to,
> + size_t len, u_char *buf, u32 op_type)
> +{
> + struct rockchip_sfc_chip_priv *priv = nor->priv;
> + struct rockchip_sfc *sfc = priv->sfc;
> + int ret;
> +
> + if (likely(sfc->use_dma))
> + return rockchip_sfc_dma_transfer(nor, from_to, len,
> + buf, op_type);
> +
> + /* Fall back to PIO mode if DMA isn't present */
> + ret = rockchip_sfc_pio_transfer(nor, from_to, len,
> + (u_char *)buf, op_type);
> + if (ret) {
> + if (op_type == SFC_CMD_DIR_RD)
> + dev_warn(nor->dev, "PIO read timeout\n");
> + else
> + dev_warn(nor->dev, "PIO write timeout\n");
> + return ret;
> + }
> +
> + return len;
> +}
> +
> +static ssize_t rockchip_sfc_read(struct spi_nor *nor, loff_t from,
> + size_t len, u_char *read_buf)
> +{
> + return rockchip_sfc_do_rd_wr(nor, from, len,
> + read_buf, SFC_CMD_DIR_RD);
> +}
> +
> +static ssize_t rockchip_sfc_write(struct spi_nor *nor, loff_t to,
> + size_t len, const u_char *write_buf)
> +{
> + return rockchip_sfc_do_rd_wr(nor, to, len,
> + (u_char *)write_buf,
> + SFC_CMD_DIR_WR);
> +}
> +
> +static int rockchip_sfc_register(struct device_node *np,
> + struct rockchip_sfc *sfc)
> +{
> + struct device *dev = sfc->dev;
> + struct mtd_info *mtd;
> + struct spi_nor *nor;
> + int ret;
> +
> + nor = &sfc->flash[sfc->num_chip].nor;
> + nor->dev = dev;
> + spi_nor_set_flash_node(nor, np);
> +
> + ret = of_property_read_u8(np, "reg", &sfc->flash[sfc->num_chip].cs);
> + if (ret) {
> + dev_err(dev, "No reg property for %s\n",
> + np->full_name);
> + return ret;
> + }
> +
> + ret = of_property_read_u32(np, "spi-max-frequency",
> + &sfc->flash[sfc->num_chip].clk_rate);
> + if (ret) {
> + dev_err(dev, "No spi-max-frequency property for %s\n",
> + np->full_name);
> + return ret;
> + }
> +
> + sfc->flash[sfc->num_chip].sfc = sfc;
> + nor->priv = &(sfc->flash[sfc->num_chip]);
> +
> + nor->prepare = rockchip_sfc_prep;
> + nor->unprepare = rockchip_sfc_unprep;
> + nor->read_reg = rockchip_sfc_read_reg;
> + nor->write_reg = rockchip_sfc_write_reg;
> + nor->read = rockchip_sfc_read;
> + nor->write = rockchip_sfc_write;
> + nor->erase = NULL;
> + ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> + if (ret)
> + return ret;
> +
> + mtd = &(nor->mtd);
> + mtd->name = np->name;
> + ret = mtd_device_register(mtd, NULL, 0);
> + if (ret)
> + return ret;
> +
> + sfc->num_chip++;
> + return 0;
> +}
> +
> +static void rockchip_sfc_unregister_all(struct rockchip_sfc *sfc)
> +{
> + int i;
> +
> + for (i = 0; i < sfc->num_chip; i++)
> + mtd_device_unregister(&sfc->flash[i].nor.mtd);
> +}
> +
> +static int rockchip_sfc_register_all(struct rockchip_sfc *sfc)
> +{
> + struct device *dev = sfc->dev;
> + struct device_node *np;
> + int ret;
> +
> + for_each_available_child_of_node(dev->of_node, np) {
> + ret = rockchip_sfc_register(np, sfc);
> + if (ret)
> + goto fail;
> +
> + if (sfc->num_chip == SFC_MAX_CHIPSELECT_NUM) {
> + dev_warn(dev, "Exceeds the max cs limitation\n");
> + break;
> + }
> + }
> +
> + return 0;
> +
> +fail:
> + dev_err(dev, "Failed to register all chips\n");
> + /* Unregister all the _registered_ nor flash */
> + rockchip_sfc_unregister_all(sfc);
> + return ret;
> +}
> +
> +static irqreturn_t rockchip_sfc_irq_handler(int irq, void *dev_id)
> +{
> + struct rockchip_sfc *sfc = dev_id;
> + u32 reg;
> +
> + reg = readl_relaxed(sfc->regbase + SFC_RISR);
> + dev_dbg(sfc->dev, "Get irq: 0x%x\n", reg);
> +
> + /* Clear interrupt */
> + writel_relaxed(reg, sfc->regbase + SFC_ICLR);
> +
> + if (reg & SFC_RISR_TRAN_FINISH)
> + complete(&sfc->cp);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int rockchip_sfc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + struct rockchip_sfc *sfc;
> + int ret;
> +
> + sfc = devm_kzalloc(dev, sizeof(*sfc), GFP_KERNEL);
> + if (!sfc)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, sfc);
> + sfc->dev = dev;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + sfc->regbase = devm_ioremap_resource(dev, res);
> + if (IS_ERR(sfc->regbase))
> + return PTR_ERR(sfc->regbase);
> +
> + sfc->clk = devm_clk_get(&pdev->dev, "sfc");
> + if (IS_ERR(sfc->clk)) {
> + dev_err(&pdev->dev, "Failed to get sfc interface clk\n");
> + return PTR_ERR(sfc->clk);
> + }
> +
> + sfc->hclk = devm_clk_get(&pdev->dev, "hsfc");
> + if (IS_ERR(sfc->hclk)) {
> + dev_err(&pdev->dev, "Failed to get sfc ahp clk\n");
> + return PTR_ERR(sfc->hclk);
> + }
> +
> + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> + if (ret) {
> + dev_warn(dev, "Unable to set dma mask\n");
> + return ret;
> + }
> +
> + sfc->buffer = dmam_alloc_coherent(dev, SFC_DMA_MAX_LEN,
> + &sfc->dma_buffer, GFP_KERNEL);
> + if (!sfc->buffer)
> + return -ENOMEM;
> +
> + mutex_init(&sfc->lock);
> +
> + ret = clk_prepare_enable(sfc->hclk);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to enable hclk\n");
> + goto err_hclk;
> + }
> +
> + ret = clk_prepare_enable(sfc->clk);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to enable clk\n");
> + goto err_clk;
> + }
> +
> + sfc->use_dma = !of_property_read_bool(sfc->dev->of_node,
> + "rockchip,sfc-no-DMA");
> +
> + sfc->negative_edge = of_device_is_compatible(sfc->dev->of_node,
> + "rockchip,rk1108-sfc");
> + /* Find the irq */
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0) {
> + dev_err(dev, "Failed to get the irq\n");
> + goto err_irq;
> + }
> +
> + ret = devm_request_irq(dev, ret, rockchip_sfc_irq_handler,
> + 0, pdev->name, sfc);
> + if (ret) {
> + dev_err(dev, "Failed to request irq\n");
> + goto err_irq;
> + }
> +
> + sfc->num_chip = 0;
> + ret = rockchip_sfc_init(sfc);
> + if (ret)
> + goto err_irq;
> +
> + pm_runtime_get_noresume(&pdev->dev);
> + pm_runtime_set_active(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
> + pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> + pm_runtime_use_autosuspend(&pdev->dev);
> +
> + ret = rockchip_sfc_register_all(sfc);
> + if (ret)
> + goto err_register;
> +
> + clk_disable_unprepare(sfc->clk);
> + pm_runtime_put_autosuspend(&pdev->dev);
> + return 0;
> +
> +err_register:
> + pm_runtime_disable(&pdev->dev);
> + pm_runtime_set_suspended(&pdev->dev);
> + pm_runtime_put_noidle(&pdev->dev);
> +err_irq:
> + clk_disable_unprepare(sfc->clk);
> +err_clk:
> + clk_disable_unprepare(sfc->hclk);
> +err_hclk:
> + mutex_destroy(&sfc->lock);
> + return ret;
> +}
> +
> +static int rockchip_sfc_remove(struct platform_device *pdev)
> +{
> + struct rockchip_sfc *sfc = platform_get_drvdata(pdev);
> +
> + pm_runtime_get_sync(&pdev->dev);
> + pm_runtime_disable(&pdev->dev);
> + pm_runtime_put_noidle(&pdev->dev);
> +
> + rockchip_sfc_unregister_all(sfc);
> + mutex_destroy(&sfc->lock);
> + clk_disable_unprepare(sfc->clk);
> + clk_disable_unprepare(sfc->hclk);
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM
> +int rockchip_sfc_runtime_suspend(struct device *dev)
> +{
> + struct rockchip_sfc *sfc = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(sfc->hclk);
> + return 0;
> +}
> +
> +int rockchip_sfc_runtime_resume(struct device *dev)
> +{
> + struct rockchip_sfc *sfc = dev_get_drvdata(dev);
> +
> + clk_prepare_enable(sfc->hclk);
> + return rockchip_sfc_reset(sfc);
> +}
> +#endif /* CONFIG_PM */
> +
> +static const struct of_device_id rockchip_sfc_dt_ids[] = {
> + { .compatible = "rockchip,sfc"},
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_sfc_dt_ids);
> +
> +static const struct dev_pm_ops rockchip_sfc_dev_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(rockchip_sfc_runtime_suspend,
> + rockchip_sfc_runtime_resume, NULL)
> +};
> +
> +static struct platform_driver rockchip_sfc_driver = {
> + .driver = {
> + .name = "rockchip-sfc",
> + .of_match_table = rockchip_sfc_dt_ids,
> + .pm = &rockchip_sfc_dev_pm_ops,
> + },
> + .probe = rockchip_sfc_probe,
> + .remove = rockchip_sfc_remove,
> +};
> +module_platform_driver(rockchip_sfc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Rockchip Serial Flash Controller Driver");
> +MODULE_AUTHOR("Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>");
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 0/4] arm64: arch_timer: Add workaround for hisilicon-161601 erratum
From: Marc Zyngier @ 2017-01-16 11:29 UTC (permalink / raw)
To: Ding Tianhong, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
oss-fOR+EgIDQEHk1uMJSBkQmQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, stuart.yoder-3arQi8VN3Tc,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linuxarm-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <e0aa52a6-8dc7-65d6-6a3a-c753b6c9538e-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On 16/01/17 10:37, Ding Tianhong wrote:
>
>
> On 2017/1/12 21:24, Ding Tianhong wrote:
>>
>> On 2017/1/12 17:11, Marc Zyngier wrote:
>>> On 12/01/17 04:23, Ding Tianhong wrote:
>>>> Hi Marc:
>>>>
>>>> How about this v7, if any suggestions very grateful.
>>>
>>> It's been less than 5 days since you posted this. I'll get to it once I
>>> finish reviewing all the other patches that are sitting in the queue
>>> right before yours.
>>>
>>
>> Ok and sorry for the noisy.
>>
>
> Hi Marc：
>
> After discussion with the chip developer, we decide to update the erratum id for this bug, so I will resend a new version
> about this, if you has start to review this v7 patch set, I think I could wait until you have finished yet. :)
This has to be a stable erratum ID, and it won't be changed once the
workaround is merged (all you'll be able to do is to add new IDs where
the same fix is applicable). So please post the revised series, and make
sure that this is the *final* ID update.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 11/37] PCI: dwc: Split pcie-designware.c into host and core files
From: Kishon Vijay Abraham I @ 2017-01-16 11:30 UTC (permalink / raw)
To: Joao Pinto, Bjorn Helgaas, Jingoo Han, Arnd Bergmann
Cc: devicetree, linux-samsung-soc, linux-doc, linux-pci, nsekhar,
linux-kernel, linux-arm-kernel, linux-arm-msm, linux-omap,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <0a16c862-312f-655e-3d66-bdb0c1f78584@synopsys.com>
Hi Joao,
On Monday 16 January 2017 03:57 PM, Joao Pinto wrote:
>
> Hi,
>
> Às 5:21 AM de 1/16/2017, Kishon Vijay Abraham I escreveu:
>> Hi Joao,
>>
>> On Friday 13 January 2017 10:19 PM, Joao Pinto wrote:
>>> Às 10:26 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
>>>> Split pcie-designware.c into pcie-designware-host.c that contains
>>>> the host specific parts of the driver and pcie-designware.c that
>>>> contains the parts used by both host driver and endpoint driver.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> ---
>>>> drivers/pci/dwc/Makefile | 2 +-
>>>> drivers/pci/dwc/pcie-designware-host.c | 619 ++++++++++++++++++++++++++++++++
>>>> drivers/pci/dwc/pcie-designware.c | 613 +------------------------------
>>>> drivers/pci/dwc/pcie-designware.h | 8 +
>>>> 4 files changed, 634 insertions(+), 608 deletions(-)
>>>> create mode 100644 drivers/pci/dwc/pcie-designware-host.c
>>>>
>>>> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
>>>> index 7d27c14..3b57e55 100644
>>>> --- a/drivers/pci/dwc/Makefile
>>>> +++ b/drivers/pci/dwc/Makefile
>>>> @@ -1,4 +1,4 @@
>>>
>>> (snip...)
>>>
>>>> -static void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>>>> - int type, u64 cpu_addr, u64 pci_addr,
>>>> - u32 size)
>>>> +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>>>> + u64 cpu_addr, u64 pci_addr, u32 size)
>>>> {
>>>> u32 retries, val;
>>>>
>>>> @@ -186,220 +151,6 @@ static void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>>>> dev_err(pci->dev, "iATU is not being enabled\n");
>>>> }
>>>
>>> Kishon, iATU only makes sense in The Root Complex (host), so it should be inside
>>> the pcie-designware-host.
>>
>> That is not true. Outbound ATU should be programmed to access host side buffers
>> and inbound ATU should be programmed for the host to access EP mem space.
>
> Sorry, I was not clear enough. What I was trying to suggest is, since the ATU
> programming is done by the host, wouldn't be better to include it in the
> pcie-designware-host? It is just an architectural detail.
ATU programming is required in EP mode. See "[PATCH 24/37] PCI: dwc:
designware: Add EP mode support" in this patch series.
Anything that's required by both EP mode and RC mode, I've placed in
pcie-designware.c
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define
From: Wolfram Sang @ 2017-01-16 11:40 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Chris Brandt, Simon Horman, Magnus Damm, Rob Herring,
Mark Rutland, devicetree@vger.kernel.org, Linux-Renesas,
Wolfram Sang
In-Reply-To: <CAMuHMdX=cNuBSMqvPKYa7J5g7T4dSfHhSjcGdmuOPw_as=6RQQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 680 bytes --]
> > and then in the code do:
> >
> > struct *cd_clk;
> > cd_clk = devm_clk_get(&pdev->dev, "cd");
> > if (cd_clk) {
> > clk_prepare_enable(cd_clk);
> > }
> >
> > (this simple 1-line fix patch is getting a lot more complicated)
>
> Disclaimer: I don't know how/if the SDHI core manages clocks, and may
> interfere. Adding Wolfram.
Thanks for the heads up.
We have special callbacks for en-/disabling clocks:
sh_mobile_sdhi_clk_enable() and sh_mobile_sdhi_clk_disable().
I think those functions should get the above if-blocks (without curly
braces) to ensure we always have consistent 00 or 11 settings.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v4 4/5] dt: bindings: i2c-mux-pca954x: Add documentation for i2c-mux-irq-mask-en
From: Peter Rosin @ 2017-01-16 11:40 UTC (permalink / raw)
To: Phil Reid, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484536275-75995-5-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
On 2017-01-16 04:11, Phil Reid wrote:
> Unfortunately some hardware device will assert their irq line immediately
> on power on and provide no mechanism to mask the irq. As the i2c muxes
> provide no method to mask irq line this provides a work around by keeping
> the parent irq masked until enough device drivers have loaded to service
> all pending interrupts.
>
> For example the the ltc1760 assert its SMBALERT irq immediately on power
> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
> device is registered irq are enabled and fire continuously as the second
> device driver has not yet loaded. Setting this parameter to 0x3 while
> delay the irq being enabled until both devices are ready.
>
> Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Hmm, I see that this is already acked by some people :-) ...
... but. I just thought a bit more about it and going forward it might
make sense to make this not a bitmask that only support 0 or 1 irq clients
for each mux segment, and instead make it one u32 for each segment. Then
the binding would extend to also cover cases where several i2c clients
register for interrupts and the shared interrupt needs to be masked until
the last client is registered.
Or is that idea over-engineered?
I'd be satisfied if the implementation simply mapped the u32 array to
a bitmask, as long as it failed noisily when finding anything bigger
than 1 in the array.
Cheers,
peda
> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
> Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> index aa09704..6de1e8e 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> @@ -19,6 +19,8 @@ Optional Properties:
> - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
> children in idle state. This is necessary for example, if there are several
> multiplexers on the bus and the devices behind them use same I2C addresses.
> + - nxp,irq-mask-enable: BitMask; Defines a mask for which irq lines need to be
> + unmasked before the parent irq line in enabled.
> - interrupt-parent: Phandle for the interrupt controller that services
> interrupts for this device.
> - interrupts: Interrupt mapping for IRQ.
> @@ -36,6 +38,7 @@ Example:
> #size-cells = <0>;
> reg = <0x74>;
>
> + nxp,irq-mask-enable = <0x3>;
> interrupt-parent = <&ipic>;
> interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
> interrupt-controller;
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [v2 2/3] ARM: dts: STM32 Add USB FS host mode support
From: Alexandre Torgue @ 2017-01-16 11:47 UTC (permalink / raw)
To: Bruno Herrera
Cc: Mark Rutland, devicetree, Russell King, linux-kernel, Rob Herring,
Maxime Coquelin, linux-arm-kernel
In-Reply-To: <CAF3+TqcD9v=Z4BAprMoWNnDddmTkzKBAb8Yz1XnHLo0_2-995g@mail.gmail.com>
On 01/16/2017 11:26 AM, Bruno Herrera wrote:
> Hi Alex,
>
> On Mon, Jan 16, 2017 at 6:57 AM, Alexandre Torgue
> <alexandre.torgue@st.com> wrote:
>> Hi Bruno,
>>
>> On 01/16/2017 03:09 AM, Bruno Herrera wrote:
>>>
>>> This patch adds the USB pins and nodes for USB HS/FS cores working at FS
>>> speed,
>>> using embedded PHY.
>>>
>>> Signed-off-by: Bruno Herrera <bruherrera@gmail.com>
>>
>>
>> Sorry, but what is patch 1 & pacth 3 status ?
>
> My bad, I'll add the status of the patch series version 3.
>>
>> For this one, can split it in 3 patches (one patch for SOC and one for each
>> board) please.
>>
>
> No problem.
>>
>>
>>> ---
>>> arch/arm/boot/dts/stm32f429-disco.dts | 30 ++++++++++++++++++++++++++++++
>>> arch/arm/boot/dts/stm32f429.dtsi | 35
>>> ++++++++++++++++++++++++++++++++++-
>>> arch/arm/boot/dts/stm32f469-disco.dts | 30 ++++++++++++++++++++++++++++++
>>> 3 files changed, 94 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/stm32f429-disco.dts
>>> b/arch/arm/boot/dts/stm32f429-disco.dts
>>> index 7d0415e..374c5ed 100644
>>> --- a/arch/arm/boot/dts/stm32f429-disco.dts
>>> +++ b/arch/arm/boot/dts/stm32f429-disco.dts
>>> @@ -88,6 +88,16 @@
>>> gpios = <&gpioa 0 0>;
>>> };
>>> };
>>> +
>>> + /* This turns on vbus for otg for host mode (dwc2) */
>>> + vcc5v_otg: vcc5v-otg-regulator {
>>> + compatible = "regulator-fixed";
>>> + gpio = <&gpioc 4 0>;
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&usbotg_pwren_h>;
>>> + regulator-name = "vcc5_host1";
>>> + regulator-always-on;
>>> + };
>>> };
>>>
>>> &clk_hse {
>>> @@ -99,3 +109,23 @@
>>> pinctrl-names = "default";
>>> status = "okay";
>>> };
>>> +
>>> +&usbotg_hs {
>>> + compatible = "st,stm32-fsotg", "snps,dwc2";
>>> + dr_mode = "host";
>>> + pinctrl-0 = <&usbotg_fs_pins_b>;
>>> + pinctrl-names = "default";
>>> + status = "okay";
>>> +};
>>> +
>>> +&pinctrl {
>>> + usb-host {
>>> + usbotg_pwren_h: usbotg-pwren-h {
>>> + pins {
>>> + pinmux = <STM32F429_PC4_FUNC_GPIO>;
>>> + bias-disable;
>>> + drive-push-pull;
>>> + };
>>> + };
>>> + };
>>> +};
>>
>>
>> Pinctrl muxing has to be defined/declared in stm32f429.dtsi
>>
> This is board specific logic and it vary from board to board, should
> it be defined here?
Pinmuxing definition is a SOC part (as it is a possibility offered by
SOC). Pinmuxing choice is board specific.
Regarding your code, it should not boot. Ex for disco:
+ gpio = <&gpiob 2 0>;
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&usbotg_pwren_h>;
+
usb-host {
>>> + usbotg_pwren_h: usbotg-pwren-h {
>>> + pins {
>>> + pinmux = <STM32F429_PB2_FUNC_GPIO>;
Indeed, you are declaring two time the pin PB2 (one time through pinctrl
and one other time through gpiolib). in strict mode you can't request 2
times the same Pin.
I assume that your driver want controls this GPIO (request/set direction
/ set, get value ...). in this case you only need to declare this part:
gpio = <&gpiob 2 0>;
The GPIO lib will deal with pinctrl framework for you.
And in this case, yes gpio declaration is board specific so this part
will be in board file.
Let me know, if I'm not enough clear.
Regards
Alex
>>
>>
>>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
>>> b/arch/arm/boot/dts/stm32f429.dtsi
>>> index e4dae0e..bc07aa8 100644
>>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>>> @@ -206,7 +206,7 @@
>>> reg = <0x40007000 0x400>;
>>> };
>>>
>>> - pin-controller {
>>> + pinctrl: pin-controller {
>>> #address-cells = <1>;
>>> #size-cells = <1>;
>>> compatible = "st,stm32f429-pinctrl";
>>> @@ -316,6 +316,30 @@
>>> };
>>> };
>>>
>>> + usbotg_fs_pins_a: usbotg_fs@0 {
>>> + pins {
>>> + pinmux =
>>> <STM32F429_PA10_FUNC_OTG_FS_ID>,
>>> +
>>> <STM32F429_PA11_FUNC_OTG_FS_DM>,
>>> +
>>> <STM32F429_PA12_FUNC_OTG_FS_DP>;
>>> + bias-disable;
>>> + drive-push-pull;
>>> + slew-rate = <2>;
>>> + };
>>> + };
>>> +
>>> + usbotg_fs_pins_b: usbotg_fs@1 {
>>> + pins {
>>> + pinmux =
>>> <STM32F429_PB12_FUNC_OTG_HS_ID>,
>>> +
>>> <STM32F429_PB14_FUNC_OTG_HS_DM>,
>>> +
>>> <STM32F429_PB15_FUNC_OTG_HS_DP>;
>>> + bias-disable;
>>> + drive-push-pull;
>>> + slew-rate = <2>;
>>> + };
>>> + };
>>> +
>>> +
>>> +
>>> usbotg_hs_pins_a: usbotg_hs@0 {
>>> pins {
>>> pinmux =
>>> <STM32F429_PH4_FUNC_OTG_HS_ULPI_NXT>,
>>> @@ -420,6 +444,15 @@
>>> status = "disabled";
>>> };
>>>
>>> + usbotg_fs: usb@50000000 {
>>> + compatible = "st,stm32f4xx-fsotg", "snps,dwc2";
>>> + reg = <0x50000000 0x40000>;
>>> + interrupts = <67>;
>>> + clocks = <&rcc 0 39>;
>>> + clock-names = "otg";
>>> + status = "disabled";
>>> + };
>>> +
>>> rng: rng@50060800 {
>>> compatible = "st,stm32-rng";
>>> reg = <0x50060800 0x400>;
>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
>>> b/arch/arm/boot/dts/stm32f469-disco.dts
>>> index 8877c00..8ae6763 100644
>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>> @@ -68,6 +68,17 @@
>>> soc {
>>> dma-ranges = <0xc0000000 0x0 0x10000000>;
>>> };
>>> +
>>> + /* This turns on vbus for otg for host mode (dwc2) */
>>> + vcc5v_otg: vcc5v-otg-regulator {
>>> + compatible = "regulator-fixed";
>>> + enable-active-high;
>>> + gpio = <&gpiob 2 0>;
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&usbotg_pwren_h>;
>>> + regulator-name = "vcc5_host1";
>>> + regulator-always-on;
>>> + };
>>> };
>>>
>>> &rcc {
>>> @@ -81,3 +92,22 @@
>>> &usart3 {
>>> status = "okay";
>>> };
>>> +
>>> +&usbotg_fs {
>>> + dr_mode = "host";
>>> + pinctrl-0 = <&usbotg_fs_pins_a>;
>>> + pinctrl-names = "default";
>>> + status = "okay";
>>> +};
>>> +
>>> +&pinctrl {
>>> + usb-host {
>>> + usbotg_pwren_h: usbotg-pwren-h {
>>> + pins {
>>> + pinmux = <STM32F429_PB2_FUNC_GPIO>;
>>> + bias-disable;
>>> + drive-push-pull;
>>> + };
>>> + };
>>> + };
>>> +};
>>
>> Same. Note that if you have 2 configuration for one feature (like it is here
>> for "usbotg_pwren_h"), you could index it. Not that I'm adding a dedidacted
>> pinctroller for stm32f469.
>>
> Sorry, but I dont know what you mean by index here.
> The usbotg_pwren_h (VBUS ENABLE) is attached in different port/pins
> for each board.
>
> Br.,
>
>
>> Br
>> Alex
>>>
>>>
>>
>>
^ permalink raw reply
* Re: [PATCH] arm64: dts: marvell: add generic-ahci compatibles for CP110 ahci
From: Mark Rutland @ 2017-01-16 12:06 UTC (permalink / raw)
To: Russell King
Cc: Gregory Clement, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Rob Herring, Catalin Marinas, Will Deacon,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Thomas Petazzoni
In-Reply-To: <E1cS72h-0004nc-Rr-eh5Bv4kxaXIk46pC+1QYvQNdhmdF6hFW@public.gmane.org>
On Fri, Jan 13, 2017 at 06:57:39PM +0000, Russell King wrote:
> Testing with an Armada 8040 board shows that adding the generic-ahci
> compatible to the CP110 AHCI nodes gets us working AHCI on the board.
> A previous patch series posted by Thomas Petazzoni was retracted when
> it was realised that the IP was supposed to be, and is, compatible
> with the standard register layout.
>
> Add this compatible.
>
> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
Assuming the above is correct this looks sane to me. FWIW:
Acked-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Mark.
> ---
> arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 3 ++-
> arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> index 464b491c7291..65ad781d7910 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
> @@ -128,7 +128,8 @@
> };
>
> cpm_sata0: sata@540000 {
> - compatible = "marvell,armada-8k-ahci";
> + compatible = "marvell,armada-8k-ahci",
> + "generic-ahci";
> reg = <0x540000 0x30000>;
> interrupts = <ICU_GRP_NSR 107 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&cpm_syscon0 1 15>;
> diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> index b10f4781d8fb..168d667d50a3 100644
> --- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
> @@ -128,7 +128,8 @@
> };
>
> cps_sata0: sata@540000 {
> - compatible = "marvell,armada-8k-ahci";
> + compatible = "marvell,armada-8k-ahci",
> + "generic-ahci";
> reg = <0x540000 0x30000>;
> interrupts = <ICU_GRP_NSR 107 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&cps_syscon0 1 15>;
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/5] phy: meson: add USB2 and USB3 PHY support for Meson GXL
From: Martin Blumenstingl @ 2017-01-16 12:07 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, khilman-rdvid1DuHRBWk0Htik3J/w,
carlo-KA+7E9HrN00dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, narmstrong-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <587C9566.7080306-l0cyMroinI0@public.gmane.org>
Hi Kishon,
thank you for taking the time to review this!
On Mon, Jan 16, 2017 at 10:41 AM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> wrote:
> Hi,
>
> On Saturday 26 November 2016 08:26 PM, Martin Blumenstingl wrote:
>> This adds two new USB PHY drivers found on Meson GXL and GXM SoCs.
>
> Please send them as separate drivers.
OK, I also just discovered that USB2 works even when not configuring
the USB3 PHY. That's another good reason for splitting these.
>> The registers for the USB2 PHY block handle a maximum of 4 ports (newer
>> SoCs may allow more ports, the driver handles this as long as the
>> register length is adjusted in the .dts). The PHY block theoretically
>> allows powering down each PHY port separately (by putting it into
>> "reset" state). Unfortunately this does not work (my board has 2 USB
>> ports, connected to port 1 and 2 of the dwc3's internal hub. When
>> leaving the third USB PHY disabled then the hub sees that a device is
>> plugged in, but it does not work: "usb usb1-port2: connect-debounce
>> failed").
>> The USB3 PHY will take care of enabling/disabling all available ports,
>> because the USB3 PHY also manages the mode of the USB2 PHYs.
>>
>> The USB3 PHY actually has three purposes:
>> - it provides the USB3 PHY
>> - it handles the OTG device/host mode detection interrupt
>> - it notifies the corresponding USB2 PHYs of the OTG mode changes
>> On GXL and GXM SoCs one references all available USB2 PHY ports in the
>> USB3 PHY because all are connected to the same USB controller (thus the
>> mode will always match). This behavior is configurable via devicetree,
>> by passing (or not passing) a list of other ("child") PHYs which should
>> be configured by the USB3 PHY.
>>
>> Unfortunately there are no datasheets available for any of these PHYs.
>> Both drivers were written by reading the reference drivers provided by
>> Amlogic and analyzing the registers on the kernel that was shipped with
>> my board.
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> ---
>> drivers/phy/Kconfig | 13 ++
>> drivers/phy/Makefile | 2 +
>> drivers/phy/phy-meson-gxl-usb2.c | 374 ++++++++++++++++++++++++++++++++++++++
>> drivers/phy/phy-meson-gxl-usb3.c | 377 +++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 766 insertions(+)
>> create mode 100644 drivers/phy/phy-meson-gxl-usb2.c
>> create mode 100644 drivers/phy/phy-meson-gxl-usb3.c
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 728e03f..ea74843 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -502,4 +502,17 @@ config PHY_MESON8B_USB2
>> and GXBB SoCs.
>> If unsure, say N.
>>
>> +config PHY_MESON_GXL_USB
>> + tristate "Meson GXL USB2 and USB3 PHY drivers"
>> + default ARCH_MESON
>> + depends on OF && (ARCH_MESON || COMPILE_TEST)
>> + depends on USB_SUPPORT
>> + select USB_COMMON
>> + select GENERIC_PHY
>> + select REGMAP_MMIO
>> + help
>> + Enable this to support the Meson USB2 and USB3 PHYs found in
>> + Meson GXL SoCs.
>> + If unsure, say N.
>> +
>> endmenu
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 0c7fdae..960a96e 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -61,3 +61,5 @@ obj-$(CONFIG_PHY_CYGNUS_PCIE) += phy-bcm-cygnus-pcie.o
>> obj-$(CONFIG_ARCH_TEGRA) += tegra/
>> obj-$(CONFIG_PHY_NS2_PCIE) += phy-bcm-ns2-pcie.o
>> obj-$(CONFIG_PHY_MESON8B_USB2) += phy-meson8b-usb2.o
>> +obj-$(CONFIG_PHY_MESON_GXL_USB) += phy-meson-gxl-usb2.o
>> +obj-$(CONFIG_PHY_MESON_GXL_USB) += phy-meson-gxl-usb3.o
>> diff --git a/drivers/phy/phy-meson-gxl-usb2.c b/drivers/phy/phy-meson-gxl-usb2.c
>> new file mode 100644
>> index 0000000..c081ce3
[snip]
>> +static struct phy *phy_meson_gxl_usb2_of_xlate(struct device *dev,
>> + struct of_phandle_args *args)
>> +{
>> + struct phy_meson_gxl_usb2_drv *priv = dev_get_drvdata(dev);
>> + int port;
>> +
>> + if (args->args_count != 1) {
>> + dev_err(dev, "Invalid number of cells in 'phy' property\n");
>> + return ERR_PTR(-ENODEV);
>> + }
>> +
>> + port = args->args[0];
>> + if (WARN_ON(port >= priv->num_ports))
>> + return ERR_PTR(-ENODEV);
>> +
>> + return priv->ports[port];
>> +}
>
> Please model every port as a sub-node and get rid of custom xlate implementation.
already done in my local tree.
I am in contact with the USB developers to get the USB2 PHYs working
(as all of them have to be turned on when powering up dwc3). The
result of how the dwc3 node with the PHY references may look like can
be seen here: [0]
>> +MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>");
>> +MODULE_DESCRIPTION("Meson GXL USB2 PHY driver");
>> +MODULE_LICENSE("GPL");
>
> GPL v2 to match with the file header.
good catch, thanks
>> diff --git a/drivers/phy/phy-meson-gxl-usb3.c b/drivers/phy/phy-meson-gxl-usb3.c
>> new file mode 100644
>> index 0000000..90a4028
>> --- /dev/null
>> +++ b/drivers/phy/phy-meson-gxl-usb3.c
>> @@ -0,0 +1,377 @@
>> +/*
>> + * Meson GXL USB3 PHY driver
>> + *
>> + * Copyright (C) 2016 Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/reset.h>
>> +#include <linux/regmap.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/usb/of.h>
>> +#include <linux/workqueue.h>
>> +
>> +#define USB_R0 0x00
>> + #define USB_R0_P30_FSEL_SHIFT 0
>> + #define USB_R0_P30_FSEL_MASK GENMASK(5, 0)
>> + #define USB_R0_P30_PHY_RESET BIT(6)
>> + #define USB_R0_P30_TEST_POWERDOWN_HSP BIT(7)
>> + #define USB_R0_P30_TEST_POWERDOWN_SSP BIT(8)
>> + #define USB_R0_P30_ACJT_LEVEL_SHIFT 9
>> + #define USB_R0_P30_ACJT_LEVEL_MASK GENMASK(13, 9)
>> + #define USB_R0_P30_TX_BOOST_LEVEL_SHIFT 14
>> + #define USB_R0_P30_TX_BOOST_LEVEL_MASK GENMASK(16, 14)
>> + #define USB_R0_P30_LANE0_TX2RX_LOOPBACK BIT(17)
>> + #define USB_R0_P30_LANE0_EXT_PCLK_REQ BIT(18)
>> + #define USB_R0_P30_PCS_RX_LOS_MASK_VAL_SHIFT 19
>> + #define USB_R0_P30_PCS_RX_LOS_MASK_VAL_MASK GENMASK(28, 19)
>> + #define USB_R0_U2D_SS_SCALEDOWN_MODE_SHIFT 29
>> + #define USB_R0_U2D_SS_SCALEDOWN_MODE_MASK GENMASK(30, 29)
>> + #define USB_R0_U2D_ACT BIT(31)
>> +
>> +#define USB_R1 0x04
>> + #define USB_R1_U3H_BIGENDIAN_GS BIT(0)
>> + #define USB_R1_U3H_PME_ENABLE BIT(1)
>> + #define USB_R1_U3H_HUB_PORT_OVERCURRENT_SHIFT 2
>> + #define USB_R1_U3H_HUB_PORT_OVERCURRENT_MASK GENMASK(6, 2)
>> + #define USB_R1_U3H_HUB_PORT_PERM_ATTACH_SHIFT 7
>> + #define USB_R1_U3H_HUB_PORT_PERM_ATTACH_MASK GENMASK(11, 7)
>> + #define USB_R1_U3H_HOST_U2_PORT_DISABLE_SHIFT 12
>> + #define USB_R1_U3H_HOST_U2_PORT_DISABLE_MASK GENMASK(15, 12)
>> + #define USB_R1_U3H_HOST_U3_PORT_DISABLE BIT(16)
>> + #define USB_R1_U3H_HOST_PORT_POWER_CONTROL_PRESENT BIT(17)
>> + #define USB_R1_U3H_HOST_MSI_ENABLE BIT(18)
>> + #define USB_R1_U3H_FLADJ_30MHZ_REG_SHIFT 19
>> + #define USB_R1_U3H_FLADJ_30MHZ_REG_MASK GENMASK(24, 19)
>> + #define USB_R1_P30_PCS_TX_SWING_FULL_SHIFT 25
>> + #define USB_R1_P30_PCS_TX_SWING_FULL_MASK GENMASK(31, 25)
>> +
>> +#define USB_R2 0x08
>> + #define USB_R2_P30_CR_DATA_IN_SHIFT 0
>> + #define USB_R2_P30_CR_DATA_IN_MASK GENMASK(15, 0)
>> + #define USB_R2_P30_CR_READ BIT(16)
>> + #define USB_R2_P30_CR_WRITE BIT(17)
>> + #define USB_R2_P30_CR_CAP_ADDR BIT(18)
>> + #define USB_R2_P30_CR_CAP_DATA BIT(19)
>> + #define USB_R2_P30_PCS_TX_DEEMPH_3P5DB_SHIFT 20
>> + #define USB_R2_P30_PCS_TX_DEEMPH_3P5DB_MASK GENMASK(25, 20)
>> + #define USB_R2_P30_PCS_TX_DEEMPH_6DB_SHIFT 26
>> + #define USB_R2_P30_PCS_TX_DEEMPH_6DB_MASK GENMASK(31, 26)
>> +
>> +#define USB_R3 0x0c
>> + #define USB_R3_P30_SSC_ENABLE BIT(0)
>> + #define USB_R3_P30_SSC_RANGE_SHIFT 1
>> + #define USB_R3_P30_SSC_RANGE_MASK GENMASK(3, 1)
>> + #define USB_R3_P30_SSC_REF_CLK_SEL_SHIFT 4
>> + #define USB_R3_P30_SSC_REF_CLK_SEL_MASK GENMASK(12, 4)
>> + #define USB_R3_P30_REF_SSP_EN BIT(13)
>> + #define USB_R3_P30_LOS_BIAS_SHIFT 16
>> + #define USB_R3_P30_LOS_BIAS_MASK GENMASK(18, 16)
>> + #define USB_R3_P30_LOS_LEVEL_SHIFT 19
>> + #define USB_R3_P30_LOS_LEVEL_MASK GENMASK(23, 19)
>> + #define USB_R3_P30_MPLL_MULTIPLIER_SHIFT 24
>> + #define USB_R3_P30_MPLL_MULTIPLIER_MASK GENMASK(30, 24)
>> +
>> +#define USB_R4 0x10
>> + #define USB_R4_P21_PORT_RESET_0 BIT(0)
>> + #define USB_R4_P21_SLEEP_M0 BIT(1)
>> + #define USB_R4_MEM_PD_SHIFT 2
>> + #define USB_R4_MEM_PD_MASK GENMASK(3, 2)
>> + #define USB_R4_P21_ONLY BIT(4)
>> +
>> +#define USB_R5 0x14
>> + #define USB_R5_ID_DIG_SYNC BIT(0)
>> + #define USB_R5_ID_DIG_REG BIT(1)
>> + #define USB_R5_ID_DIG_CFG_SHIFT 2
>> + #define USB_R5_ID_DIG_CFG_MASK GENMASK(3, 2)
>> + #define USB_R5_ID_DIG_EN_0 BIT(4)
>> + #define USB_R5_ID_DIG_EN_1 BIT(5)
>> + #define USB_R5_ID_DIG_CURR BIT(6)
>> + #define USB_R5_ID_DIG_IRQ BIT(7)
>> + #define USB_R5_ID_DIG_TH_SHIFT 8
>> + #define USB_R5_ID_DIG_TH_MASK GENMASK(15, 8)
>> + #define USB_R5_ID_DIG_CNT_SHIFT 16
>> + #define USB_R5_ID_DIG_CNT_MASK GENMASK(23, 16)
>> +
>> +/* read-only register */
>> +#define USB_R6 0x18
>> + #define USB_R6_P30_CR_DATA_OUT_SHIFT 0
>> + #define USB_R6_P30_CR_DATA_OUT_MASK GENMASK(15, 0)
>> + #define USB_R6_P30_CR_ACK BIT(16)
>> +
>> +#define RESET_COMPLETE_TIME 500
>> +
>> +struct phy_meson_gxl_usb3_priv {
>> + struct regmap *regmap;
>> + struct delayed_work otg_work;
>> + struct phy *this_phy;
>> + int num_usb2_phys;
>> + struct phy **usb2_phys;
>> +};
>> +
>> +static const struct regmap_config phy_meson_gxl_usb3_regmap_conf = {
>> + .reg_bits = 32,
>> + .val_bits = 32,
>> + .reg_stride = 4,
>> + .max_register = USB_R6,
>> +};
>> +
>> +static int phy_meson_gxl_usb3_update_mode(struct phy *phy)
>> +{
>> + struct phy_meson_gxl_usb3_priv *priv = phy_get_drvdata(phy);
>> + u32 val;
>> + enum phy_mode mode;
>> + int i, ret;
>> +
>> + ret = regmap_read(priv->regmap, USB_R5, &val);
>> + if (ret)
>> + return ret;
>> +
>> + if (val & USB_R5_ID_DIG_CURR) {
>> + mode = PHY_MODE_USB_DEVICE;
>> +
>> + regmap_update_bits(priv->regmap, USB_R0, USB_R0_U2D_ACT,
>> + USB_R0_U2D_ACT);
>> + regmap_update_bits(priv->regmap, USB_R4, USB_R4_P21_SLEEP_M0,
>> + USB_R4_P21_SLEEP_M0);
>> + } else {
>> + mode = PHY_MODE_USB_HOST;
>> +
>> + regmap_update_bits(priv->regmap, USB_R0, USB_R0_U2D_ACT, 0);
>> + regmap_update_bits(priv->regmap, USB_R4, USB_R4_P21_SLEEP_M0,
>> + 0);
>> + }
>> +
>> + /* inform the USB2 PHY that we have changed the mode */
>> + for (i = 0; i < priv->num_usb2_phys; i++) {
>> + ret = phy_set_mode(priv->usb2_phys[i], mode);
>
> I'm finding it difficult to understand this. Why should the mode of one phy be
> set from another phy? Maybe this part should be implemented using extcon?
sounds like a good idea, I will postpone the USB3 PHY driver though
since it's currently only used for "mode switching" (between USB
host/device) and the whole thing requires MUCH more work (as a dwc2
controller is used for device mode, while a dwc3 controller is used
for host mode).
>> + if (ret) {
>> + dev_err(&phy->dev,
>> + "Failed to update usb2-phy #%d mode to %d\n",
>> + i, mode);
>> + return ret;
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static void phy_meson_gxl_usb3_work(struct work_struct *data)
>> +{
>> + struct phy_meson_gxl_usb3_priv *priv =
>> + container_of(data, struct phy_meson_gxl_usb3_priv,
>> + otg_work.work);
>> +
>> + phy_meson_gxl_usb3_update_mode(priv->this_phy);
>> +
>> + /* unmask IRQs which may have arrived in the meantime */
>> + regmap_update_bits(priv->regmap, USB_R5, USB_R5_ID_DIG_IRQ, 0);
>> +}
>> +
>> +static int phy_meson_gxl_usb3_init(struct phy *phy)
>> +{
>> + struct phy_meson_gxl_usb3_priv *priv = phy_get_drvdata(phy);
>> + int i, ret;
>> +
>> + for (i = 0; i < priv->num_usb2_phys; i++) {
>> + ret = phy_init(priv->usb2_phys[i]);
>> + if (ret) {
>> + dev_err(&phy->dev,
>> + "Failed to initialize related usb2-phy #%d\n",
>> + i);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int phy_meson_gxl_usb3_exit(struct phy *phy)
>> +{
>> + struct phy_meson_gxl_usb3_priv *priv = phy_get_drvdata(phy);
>> + int i, ret;
>> +
>> + for (i = 0; i < priv->num_usb2_phys; i++) {
>> + ret = phy_exit(priv->usb2_phys[i]);
>> + if (ret) {
>> + dev_err(&phy->dev,
>> + "Failed to exit related usb2-phy #%d\n", i);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int phy_meson_gxl_usb3_power_on(struct phy *phy)
>> +{
>> + struct phy_meson_gxl_usb3_priv *priv = phy_get_drvdata(phy);
>> + int i, ret;
>> +
>> + for (i = 0; i < priv->num_usb2_phys; i++) {
>> + ret = phy_power_on(priv->usb2_phys[i]);
>> + if (ret) {
>> + dev_err(&phy->dev,
>> + "Failed to power on related usb2-phy #%d\n",
>> + i);
>> + return ret;
>> + }
>> + }
>> +
>> + regmap_update_bits(priv->regmap, USB_R1,
>> + USB_R1_U3H_FLADJ_30MHZ_REG_MASK,
>> + 0x20 << USB_R1_U3H_FLADJ_30MHZ_REG_SHIFT);
>> +
>> + regmap_update_bits(priv->regmap, USB_R5, USB_R5_ID_DIG_EN_0,
>> + USB_R5_ID_DIG_EN_0);
>> + regmap_update_bits(priv->regmap, USB_R5, USB_R5_ID_DIG_EN_1,
>> + USB_R5_ID_DIG_EN_1);
>> + regmap_update_bits(priv->regmap, USB_R5, USB_R5_ID_DIG_TH_MASK,
>> + 0xff << USB_R5_ID_DIG_TH_SHIFT);
>> +
>> + return phy_meson_gxl_usb3_update_mode(phy);
>> +}
>> +
>> +static int phy_meson_gxl_usb3_power_off(struct phy *phy)
>> +{
>> + struct phy_meson_gxl_usb3_priv *priv = phy_get_drvdata(phy);
>> + int i, ret;
>> +
>> + for (i = 0; i < priv->num_usb2_phys; i++) {
>> + ret = phy_power_off(priv->usb2_phys[i]);
>> + if (ret) {
>> + dev_err(&phy->dev,
>> + "Failed to power off related usb2-phy #%d\n",
>> + i);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t phy_meson_gxl_usb3_irq(int irq, void *data)
>> +{
>> + u32 val;
>> + struct phy_meson_gxl_usb3_priv *priv = data;
>> +
>> + regmap_read(priv->regmap, USB_R5, &val);
>> + if (!(val & USB_R5_ID_DIG_IRQ)) {
>> + dev_err(&priv->this_phy->dev, "spurious interrupt\n");
>> + return IRQ_NONE;
>> + }
>> +
>> + schedule_delayed_work(&priv->otg_work, msecs_to_jiffies(10));
>> +
>> + /* acknowledge the IRQ */
>> + regmap_update_bits(priv->regmap, USB_R5, USB_R5_ID_DIG_IRQ, 0);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static const struct phy_ops phy_meson_gxl_usb3_ops = {
>> + .init = phy_meson_gxl_usb3_init,
>> + .exit = phy_meson_gxl_usb3_exit,
>> + .power_on = phy_meson_gxl_usb3_power_on,
>> + .power_off = phy_meson_gxl_usb3_power_off,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static int phy_meson_gxl_usb3_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> + struct phy_meson_gxl_usb3_priv *priv;
>> + struct resource *res;
>> + struct phy *phy;
>> + struct phy_provider *phy_provider;
>> + void __iomem *base;
>> + int i, irq;
>> +
>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + priv->regmap = devm_regmap_init_mmio(dev, base,
>> + &phy_meson_gxl_usb3_regmap_conf);
>> + if (IS_ERR(priv->regmap))
>> + return PTR_ERR(priv->regmap);
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + if (irq >= 0) {
>> + INIT_DELAYED_WORK(&priv->otg_work, phy_meson_gxl_usb3_work);
>> +
>> + irq = devm_request_irq(dev, irq, phy_meson_gxl_usb3_irq,
>> + IRQF_SHARED, dev_name(dev),
>> + priv);
>> + if (irq < 0) {
>> + dev_err(dev, "could not register IRQ handler (%d)\n",
>> + irq);
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + priv->num_usb2_phys = of_count_phandle_with_args(np, "phys",
>> + "#phy-cells");
>> +
>> + priv->usb2_phys = devm_kcalloc(dev, priv->num_usb2_phys,
>> + sizeof(*priv->usb2_phys), GFP_KERNEL);
>> + if (!priv->usb2_phys)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < priv->num_usb2_phys; i++) {
>> + priv->usb2_phys[i] = devm_of_phy_get_by_index(dev, np, i);
>
> I'm not sure if referencing usb2_phy from here is the right approach.
that would probably be gone with the USB patches from [0] and with the
switch to extcon
I will send an updated version once we know how to handle powering up
the PHY in xhci-plat.c (which is the series from [0]).
Regards,
Martin
[0] http://marc.info/?l=linux-usb&m=148414866203601&w=2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 5/5] i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs
From: Peter Rosin @ 2017-01-16 12:08 UTC (permalink / raw)
To: Phil Reid, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484536275-75995-6-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
On 2017-01-16 04:11, Phil Reid wrote:
> Unfortunately some hardware device will assert their irq line immediately
> on power on and provide no mechanism to mask the irq. As the i2c muxes
> provide no method to mask irq line this provides a work around by keeping
> the parent irq masked until enough device drivers have loaded to service
> all pending interrupts.
>
> For example the the ltc1760 assert its SMBALERT irq immediately on power
> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
> device is registered irq are enabled and fire continuously as the second
> device driver has not yet loaded. Setting this parameter to 0x3 while
> delay the irq being enabled until both devices are ready.
>
> Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
> drivers/i2c/muxes/i2c-mux-pca954x.c | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index f55da88..66f7ed8 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -76,6 +76,19 @@ struct chip_desc {
> } muxtype;
> };
>
> +/*
> + * irq_mask_enable: Provides a mechanism to work around hardware that asserts
> + * their irq immediately on power on. It allows the enabling of the irq to be
> + * delayed until the corresponding bits in the the irq_mask are set thru
> + * irq_unmask.
> + * For example the ltc1760 assert its SMBALERT irq immediately on power on.
> + * With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
> + * device is registered irq are enabled and fire continuously as the second
> + * device driver has not yet loaded. Setting this parameter to 0x3 while
> + * delay the irq being enabled until both devices are ready.
> + * This workaround will not work if two devices share an interrupt on the
> + * same bus segment.
> + */
> struct pca954x {
> const struct chip_desc *chip;
>
> @@ -84,6 +97,7 @@ struct pca954x {
> struct i2c_client *client;
>
> struct irq_domain *irq;
> + unsigned int irq_mask_enable;
> unsigned int irq_mask;
> spinlock_t lock;
> };
> @@ -280,9 +294,12 @@ static void pca954x_irq_unmask(struct irq_data *idata)
>
> spin_lock_irqsave(&data->lock, flags);
>
> - if (!data->irq_mask)
> + if (!data->irq_mask_enable && !data->irq_mask)
> enable_irq(data->client->irq);
> data->irq_mask |= BIT(pos);
> + if (data->irq_mask_enable &&
> + (data->irq_mask & data->irq_mask) == data->irq_mask_enable)
Hmm, I see that some apparently incompetent person :-) already acked this,
but the (data->irq_mask & data->irq_mask) part doesn't make sense at all.
> + enable_irq(data->client->irq);
>
Hmm2, if you have a problematic device (like the ltc1760) on mux segment 0
and sane devices on other segments I'd be inclined to specify irq-mask-enable
as 0x1. But then this is possible:
1. ltc1760 registers its irq
2. enable_irq(data->client->irq) is called because irq_mask_enable is "fulfilled"
3. a sane irq register an irq on some other segment
4. enable_irq(...) is called again (which the code appears to try to avoid)
As I read the code, there will be problems with specifying irq-mask-enable
whenever there are more than one irq-client on a mux segment.
So, I'm removing my ack until the above is resolved...
Cheers,
peda
> spin_unlock_irqrestore(&data->lock, flags);
> }
> @@ -409,6 +426,9 @@ static int pca954x_probe(struct i2c_client *client,
> idle_disconnect_dt = of_node &&
> of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
>
> + of_property_read_u32(of_node, "nxp,irq-mask-enable",
> + &data->irq_mask_enable);
> +
> ret = pca954x_irq_setup(muxc);
> if (ret)
> goto fail_del_adapters;
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 01/24] [media] dt-bindings: Add bindings for i.MX media driver
From: Philipp Zabel @ 2017-01-16 12:09 UTC (permalink / raw)
To: Steve Longerbeam
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
fabio.estevam-3arQi8VN3Tc, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
mchehab-DgEjT+Ai2ygdnm+yROfE0A, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
nick-gcszYUEDH4VrovVCs/uTlw, markus.heiser-O6JHGLzbNUwb1SvskN2V4Q,
laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw,
bparrot-l0cyMroinI0, geert-Td1EMuHUCqxL1ZNQvxDV9g,
arnd-r2nGTMty4D4, sudipm.mukherjee-Re5JQEeQqe8AvxtiuMwx3w,
minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w,
tiffany.lin-NuS5LvNUpcJWk0Htik3J/w,
jean-christophe.trotin-qxv4g6HH51o,
horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ,
niklas.soderlund+renesas-1zkq55x86MTxsAP9Fp7wbw,
robert.jarzmik-GANU6spQydw, songjun.wu-UWL1GkI3JZL3oGB3hsPCZA,
andrew-ct.chen-NuS5LvNUpcJWk0Htik3J/w,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-media-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, Steve Longerbeam
In-Reply-To: <e609fd03-a546-330c-ec89-de1844d1b46f-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, 2017-01-13 at 11:03 -0800, Steve Longerbeam wrote:
>
> On 01/13/2017 03:55 AM, Philipp Zabel wrote:
> > Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> >> Add bindings documentation for the i.MX media driver.
> >>
> >> Signed-off-by: Steve Longerbeam <steve_longerbeam-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> >> ---
> >> Documentation/devicetree/bindings/media/imx.txt | 57 +++++++++++++++++++++++++
> >> 1 file changed, 57 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/media/imx.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/imx.txt b/Documentation/devicetree/bindings/media/imx.txt
> >> new file mode 100644
> >> index 0000000..254b64a
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/imx.txt
> >> @@ -0,0 +1,57 @@
> >> +Freescale i.MX Media Video Devices
> >> +
> >> +Video Media Controller node
> >> +---------------------------
> >> +
> >> +This is the parent media controller node for video capture support.
> >> +
> >> +Required properties:
> >> +- compatible : "fsl,imx-media";
> > Would you be opposed to calling this "capture-subsystem" instead of
> > "imx-media"? We already use "fsl,imx-display-subsystem" and
> > "fsl,imx-gpu-subsystem" for the display and GPU compound devices.
>
> sure. Some pie-in-the-sky day when DRM and media are unified,
> there could be a single device that handles them all,
Indeed :)
> but for now
> I'm fine with "fsl,capture-subsystem".
Actually, I meant fsl,imx-capture-subsystem. fsl,imx-media-subsystem
would be fine, too. Either way, I'll be happy if it looks similar to the
other two.
[...]
> > This is a clever method to get better frame timestamps. Too bad about
> > the routing requirements. Can this be used on Nitrogen6X?
>
> Absolutely, this support just needs use of the input-capture channels in the
> imx GPT. I still need to submit the patch to the imx-gpt driver that adds an
> input capture API, so at this point fsl,input-capture-channel has no effect,
> but it does work (tested on SabreAuto).
Nice.
[...]
> >> +Required properties:
> >> +- compatible : "fsl,imx6-mipi-csi2";
> > I think this should get an additional "snps,dw-mipi-csi2" compatible,
> > since the only i.MX6 specific part is the bolted-on IPU2CSI gasket.
>
> right, minus the gasket it's a Synopsys core. I'll add that compatible flag.
> Or should wait until the day this subdev is exported for general use, after
> pulling out the gasket specifics?
It can be added right away.
> >> +- reg : physical base address and length of the register set;
> >> +- clocks : the MIPI CSI-2 receiver requires three clocks: hsi_tx
> >> + (the DPHY clock), video_27m, and eim_sel;
> > Note that hsi_tx is incorrectly named. CCGR3[CG8] just happens to be the
> > shared gate bit that gates the HSI clocks as well as the MIPI
> > "ac_clk_125m", "cfg_clk", "ips_clk", and "pll_refclk" inputs to the mipi
> > csi-2 core, but we are missing shared gate clocks in the clock tree for
> > these.
>
> Yes, so many clocks for the MIPI core. Why so many? I would think
> there would need to be at most three: a clock for the MIPI CSI-2 core
> and HSI core, and a clock for the D-PHY (oh and maybe a clock for an
> M-PHY if there is one). I have no clue what all these other clocks are.
> But anyway, a single gating bit, CCGR3[CG8], seems to enable them all.
I would imagine the CSI-2 core has a high-speed clock input from the
D-PHY for serial input, an APB clock for register access (ips_clk), and
a pixel clock input for the parallel output (pixel_clk), at least.
The D-PHY will have a PLL reference input (pll_refclk?) and probably its
own register clock (cfg_clk?).
I've looked at the MIPI DSI chapter, and it looks like ac_clk_125m is
used for DSI only.
> > Both cfg_clk and pll_refclk are sourced from video_27m, so "cfg" ->
> > video_27m seems fine.
> > But I don't get "dphy".
>
> I presume it's the clock for the D-PHY.
>
> > Which input clock would that correspond to?
> > "pll_refclk?"
>
> the mux at CDCDR says it comes from PLL3_120M, or PLL2_PFD2.
I think that makes sense.
regards
Philipp
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox