* Re: [PATCH v2 1/2] pinctrl: Allow a device to indicate when to force a state
From: Tony Lindgren @ 2017-11-29 17:01 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-gpio, Linus Walleij, Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, ckeepax, ckeepax, swarren, andy.shevchenko, alcooperx,
bcm-kernel-feedback-list
In-Reply-To: <20171102231551.16220-2-f.fainelli@gmail.com>
* Florian Fainelli <f.fainelli@gmail.com> [171102 23:18]:
> It may happen that a device needs to force applying a state, e.g:
> because it only defines one state of pin states (default) but loses
> power/register contents when entering low power modes. Add a
> pinctrl_dev::flags bitmask to help describe future quirks and define
> PINCTRL_FLG_FORCE_STATE as such a settable flag.
It makes sense to tag the existing state with the context loss
information as otherwise we'll be duplicating the state in the
pinctrl driver potentially for hundreds of pins.
Maybe this patch description should clarify that it's the
pinctrl device restoring the pin state, not the pinctrl
consumer devices?
So maybe just "a pinctrl device needs to force apply a state"
instead of just device above?
Regards,
Tony
^ permalink raw reply
* Re: [PATCH V2 5/9] PCI: Export pci_flags
From: Bjorn Helgaas @ 2017-11-29 17:01 UTC (permalink / raw)
To: Manikanta Maddireddy
Cc: thierry.reding, jonathanh, robh+dt, frowand.list, bhelgaas, rjw,
tglx, vidyas, kthota, linux-tegra, devicetree, linux-pci,
linux-pm, Lorenzo Pieralisi
In-Reply-To: <1511638333-22951-6-git-send-email-mmaddireddy@nvidia.com>
[+cc Lorenzo]
On Sun, Nov 26, 2017 at 01:02:09AM +0530, Manikanta Maddireddy wrote:
> pci_flags variable is used in inline functions in 'pci.h', Tegra PCIe
> driver use one of these functions pci_add_flags() and includes 'pci.h'.
> Export pci_flags to allow Tegra PCIe host controller driver to be
> compiled as loadable kernel module.
Here's the usage in tegra_pcie_probe():
pci_add_flags(PCI_REASSIGN_ALL_RSRC | PCI_REASSIGN_ALL_BUS);
We've probably had this discussion before, but I don't know why Tegra
needs PCI_REASSIGN_ALL_RSRC and PCI_REASSIGN_ALL_BUS.
I would prefer to drop this usage of pci_add_flags() if possible. It
seems to be just an arm/powerpc thing and I'm not convinced it's
really necessary.
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> ---
> V2:
> * commit message update
>
> drivers/pci/setup-bus.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index b1ad466199ad..3567e1c4e340 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -29,6 +29,7 @@
> #include "pci.h"
>
> unsigned int pci_flags;
> +EXPORT_SYMBOL(pci_flags);
>
> struct pci_dev_resource {
> struct list_head list;
> --
> 2.1.4
>
^ permalink raw reply
* Re: [PATCH V2 6/9] PCI: tegra: free resources on probe failure
From: Bjorn Helgaas @ 2017-11-29 17:02 UTC (permalink / raw)
To: Manikanta Maddireddy
Cc: thierry.reding, jonathanh, robh+dt, frowand.list, bhelgaas, rjw,
tglx, vidyas, kthota, linux-tegra, devicetree, linux-pci,
linux-pm
In-Reply-To: <1511638333-22951-7-git-send-email-mmaddireddy@nvidia.com>
Please capitalize the first word of the changelog summary so it
matches the rest of your series (and the rest of drivers/pci history).
On Sun, Nov 26, 2017 at 01:02:10AM +0530, Manikanta Maddireddy wrote:
> tegra_pcie_probe() can fail in multiple instances, this patch takes care
> of freeing the resources which are allocated before probe fail.
>
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
^ permalink raw reply
* Re: [PATCH v2 2/2] pinctrl: Allow indicating loss of pin states during low-power
From: Tony Lindgren @ 2017-11-29 17:02 UTC (permalink / raw)
To: Linus Walleij
Cc: Florian Fainelli, linux-gpio-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, Charles Keepax, Charles Keepax, Stephen Warren,
Andy Shevchenko, Al Cooper, bcm-kernel-feedback-list
In-Reply-To: <CACRpkdZtFRB_iy1bDPZ0wkK0jf7pkTGtbZG4gQUJVR+eiO+dhA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
* Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [171129 13:03]:
> On Fri, Nov 3, 2017 at 12:15 AM, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> > Some platforms (e.g: Broadcom STB: BMIPS_GENERIC/ARCH_BRCMSTB) will lose
> > their register contents when entering their lower power state. In such a
> > case, the pinctrl-single driver that is used will not be able to restore
> > the power states without telling the core about it and having
> > pinctrl_select_state() check for that.
> >
> > This patch adds a new optional boolean property that Device Tree can
> > define in order to obtain exactly that and having the core pinctrl code
> > take that into account.
> >
> > Signed-off-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Florian, I'm really sorry for losing track of this patch set, it's
> important stuff and I see why systems are dependent on something
> like this.
>
> Tony: can you look at this from a pinctrl-single point of view?
> This is the intended consumer: pinctrl-single users that lose the
> hardware state over suspend/resume.
>
> How do you see this working with other pinctrl-single users?
Hmm well typically a device driver that loses it's context just does
save and restore of the registers in runtime PM suspend/resume
as needed. In this case it would mean duplicating the state for
potentially for hundreds of registers.. So using the existing
state in the pinctrl subsystem totally makes sense for the pins.
Florian do you have other reasons why this should be done in the
pinctrl framework instead of the driver? Might be worth describing
the reasoning in the patch descriptions :)
So as long as the pinctrl framework state is used to restore the
state by the pinctrl driver instead of the pinctrl consumer drivers,
I don't have issues with this patchset. So probably just improving
the patch messages a bit should do it.
FYI, on omaps, the PRCM hardware saves and restores the pinctrl
state so this has not been so far an issue.
Regards,
Tony
--
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 0/8] Remove use of "gpio-reset" from DT
From: Andrew F. Davis @ 2017-11-29 17:13 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Shawn Guo, Sascha Hauer
Cc: devicetree, alsa-devel, linux-kernel, Andrew F . Davis
Hello all,
I was working on fixing up the tlv320aic31xx driver when I noticed
its gpio reset was not working, it was due to this driver looking
for "gpio-reset" instead of the usual "reset-gpio". A quick check
shows this mistake is rare and only copied by one other audio CODECs:
$ git grep "reset-gpio" | wc -l
630
$ git grep "gpio-reset" | wc -l
6
Luckliy the two effected drivers only use this reset line when power
has been cut to the device, so not only were these optional
properties but they had no real functional effect anyway. Lets
just fix this before is spreads to drivers were it matters.
I've also added fixes tags to each patch so it can be individually
back-ported to where this bug was introduced if one wanted to. I'm
hoping this can sit on next for a while to get the most testing,
just in case I'm wrong about this not breaking anything.
Thanks,
Andrew
Changes from v1:
- Move old property to "Deprecated properties" section in binding
- Keep backwards compatibility for now in driver with warning
- Re-order patches to keep series bisectable
- Use "reset-gpios" over "reset-gpio"
- Drop taken patch
- Rebased on v4.15-rc1
Andrew F. Davis (8):
ASoC: tlv320aic31xx: Fix typo in DT binding documentation
ASoC: tlv320aic3x: Fix typo in DT binding documentation
ASoC: tlv320aic31xx: Fix the reset GPIO OF name
ASoC: tlv320aic3x: Fix the reset GPIO OF name
ARM: dts: am335x-pepper: Fix the audio CODEC's reset pin
ARM: dts: imx6: RDU2: Fix the audio CODEC's reset pin
ARM: dts: imx: Fix the audio CODEC's reset pin
ARM: dts: omap3-n900: Fix the audio CODEC's reset pin
Documentation/devicetree/bindings/sound/tlv320aic31xx.txt | 9 ++++++++-
Documentation/devicetree/bindings/sound/tlv320aic3x.txt | 10 +++++++++-
arch/arm/boot/dts/am335x-pepper.dts | 2 +-
arch/arm/boot/dts/imx6qdl-gw5903.dtsi | 2 +-
arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi | 4 ++--
arch/arm/boot/dts/omap3-n900.dts | 4 ++--
sound/soc/codecs/tlv320aic31xx.c | 11 +++++++++--
sound/soc/codecs/tlv320aic3x.c | 15 +++++++++++----
8 files changed, 43 insertions(+), 14 deletions(-)
--
2.15.0
^ permalink raw reply
* [PATCH v2 1/8] ASoC: tlv320aic31xx: Fix typo in DT binding documentation
From: Andrew F. Davis @ 2017-11-29 17:13 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Shawn Guo, Sascha Hauer
Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew F . Davis
In-Reply-To: <20171129171359.29382-1-afd-l0cyMroinI0@public.gmane.org>
The property used to specify a GPIO intended for reset is "reset-gpios",
this binding uses "gpio-reset", as almost all other bindings use the
former name this use of the latter was certainly not intended. It is not
compatible with newer methods used to fetch GPIO pins and to prevent the
spread of this error to other bindings lets fix this here.
We also standardize the pin as active-low, different device trees have
marked the GPIO different ways, luckily the driver currently uses the
low-level GPIO set function which does not respect the active-low flag,
but future changes may change this. This is an active-low reset, mark
it as such.
Lastly, add an example of use for this property.
Fixes: e00447fafbf7 ("ASoC: tlv320aic31xx: Add basic codec driver implementation")
Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
---
Documentation/devicetree/bindings/sound/tlv320aic31xx.txt | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt b/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt
index 6fbba562eaa7..5b3c33bb99e5 100644
--- a/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt
+++ b/Documentation/devicetree/bindings/sound/tlv320aic31xx.txt
@@ -22,7 +22,7 @@ Required properties:
Optional properties:
-- gpio-reset - gpio pin number used for codec reset
+- reset-gpios - GPIO specification for the active low RESET input.
- ai31xx-micbias-vg - MicBias Voltage setting
1 or MICBIAS_2_0V - MICBIAS output is powered to 2.0V
2 or MICBIAS_2_5V - MICBIAS output is powered to 2.5V
@@ -30,6 +30,10 @@ Optional properties:
If this node is not mentioned or if the value is unknown, then
micbias is set to 2.0V.
+Deprecated properties:
+
+- gpio-reset - gpio pin number used for codec reset
+
CODEC output pins:
* HPL
* HPR
@@ -48,6 +52,7 @@ CODEC input pins:
The pins can be used in referring sound node's audio-routing property.
Example:
+#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/sound/tlv320aic31xx-micbias.h>
tlv320aic31xx: tlv320aic31xx@18 {
@@ -56,6 +61,8 @@ tlv320aic31xx: tlv320aic31xx@18 {
ai31xx-micbias-vg = <MICBIAS_OFF>;
+ reset-gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
+
HPVDD-supply = <®ulator>;
SPRVDD-supply = <®ulator>;
SPLVDD-supply = <®ulator>;
--
2.15.0
--
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 related
* [PATCH v2 2/8] ASoC: tlv320aic3x: Fix typo in DT binding documentation
From: Andrew F. Davis @ 2017-11-29 17:13 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Shawn Guo, Sascha Hauer
Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew F . Davis
In-Reply-To: <20171129171359.29382-1-afd-l0cyMroinI0@public.gmane.org>
The property used to specify a GPIO intended for reset is "reset-gpios",
this binding uses "gpio-reset", as almost all other bindings use the
former name this use of the latter was certainly not intended. It is not
compatible with newer methods used to fetch GPIO pins and to prevent the
spread of this error to other bindings lets fix this here.
We also standardize the pin as active-low, different device trees have
marked the GPIO different ways, luckily the driver currently uses the
low-level GPIO set function which does not respect the active-low flag,
but future changes may change this. This is an active-low reset, mark
it as such.
Lastly, add an example of use for this property.
Fixes: c24fdc886fde ("ASoC: tlv320aic3x: Add device tree bindings")
Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
---
Documentation/devicetree/bindings/sound/tlv320aic3x.txt | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/tlv320aic3x.txt b/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
index ba5b45c483f5..9796c4639262 100644
--- a/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
+++ b/Documentation/devicetree/bindings/sound/tlv320aic3x.txt
@@ -17,7 +17,7 @@ Required properties:
Optional properties:
-- gpio-reset - gpio pin number used for codec reset
+- reset-gpios - GPIO specification for the active low RESET input.
- ai3x-gpio-func - <array of 2 int> - AIC3X_GPIO1 & AIC3X_GPIO2 Functionality
- Not supported on tlv320aic3104
- ai3x-micbias-vg - MicBias Voltage required.
@@ -34,6 +34,10 @@ Optional properties:
- AVDD-supply, IOVDD-supply, DRVDD-supply, DVDD-supply : power supplies for the
device as covered in Documentation/devicetree/bindings/regulator/regulator.txt
+Deprecated properties:
+
+- gpio-reset - gpio pin number used for codec reset
+
CODEC output pins:
* LLOUT
* RLOUT
@@ -61,10 +65,14 @@ The pins can be used in referring sound node's audio-routing property.
Example:
+#include <dt-bindings/gpio/gpio.h>
+
tlv320aic3x: tlv320aic3x@1b {
compatible = "ti,tlv320aic3x";
reg = <0x1b>;
+ reset-gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
+
AVDD-supply = <®ulator>;
IOVDD-supply = <®ulator>;
DRVDD-supply = <®ulator>;
--
2.15.0
--
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 related
* [PATCH v2 3/8] ASoC: tlv320aic31xx: Fix the reset GPIO OF name
From: Andrew F. Davis @ 2017-11-29 17:13 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Shawn Guo, Sascha Hauer
Cc: devicetree, alsa-devel, linux-kernel, Andrew F . Davis
In-Reply-To: <20171129171359.29382-1-afd@ti.com>
The correct DT property for specifying a GPIO used for reset
is "reset-gpios", fix this here.
Fixes: e00447fafbf7 ("ASoC: tlv320aic31xx: Add basic codec driver implementation")
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
sound/soc/codecs/tlv320aic31xx.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic31xx.c b/sound/soc/codecs/tlv320aic31xx.c
index e2862372c26e..4837f25b0760 100644
--- a/sound/soc/codecs/tlv320aic31xx.c
+++ b/sound/soc/codecs/tlv320aic31xx.c
@@ -1279,9 +1279,16 @@ static void aic31xx_pdata_from_of(struct aic31xx_priv *aic31xx)
aic31xx->pdata.micbias_vg = MICBIAS_2_0V;
}
- ret = of_get_named_gpio(np, "gpio-reset", 0);
- if (ret > 0)
+ ret = of_get_named_gpio(np, "reset-gpios", 0);
+ if (ret > 0) {
aic31xx->pdata.gpio_reset = ret;
+ } else {
+ ret = of_get_named_gpio(np, "gpio-reset", 0);
+ if (ret > 0) {
+ dev_warn(aic31xx->dev, "Using deprecated property \"gpio-reset\", please update your DT");
+ aic31xx->pdata.gpio_reset = ret;
+ }
+ }
}
#else /* CONFIG_OF */
static void aic31xx_pdata_from_of(struct aic31xx_priv *aic31xx)
--
2.15.0
^ permalink raw reply related
* [PATCH v2 4/8] ASoC: tlv320aic3x: Fix the reset GPIO OF name
From: Andrew F. Davis @ 2017-11-29 17:13 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Shawn Guo, Sascha Hauer
Cc: devicetree, alsa-devel, linux-kernel, Andrew F . Davis
In-Reply-To: <20171129171359.29382-1-afd@ti.com>
The correct DT property for specifying a GPIO used for reset
is "reset-gpios", fix this here.
Fixes: c24fdc886fde ("ASoC: tlv320aic3x: Add device tree bindings")
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
sound/soc/codecs/tlv320aic3x.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index 06f92571eba4..b751cad545da 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -1804,11 +1804,18 @@ static int aic3x_i2c_probe(struct i2c_client *i2c,
if (!ai3x_setup)
return -ENOMEM;
- ret = of_get_named_gpio(np, "gpio-reset", 0);
- if (ret >= 0)
+ ret = of_get_named_gpio(np, "reset-gpios", 0);
+ if (ret >= 0) {
aic3x->gpio_reset = ret;
- else
- aic3x->gpio_reset = -1;
+ } else {
+ ret = of_get_named_gpio(np, "gpio-reset", 0);
+ if (ret > 0) {
+ dev_warn(&i2c->dev, "Using deprecated property \"gpio-reset\", please update your DT");
+ aic3x->gpio_reset = ret;
+ } else {
+ aic3x->gpio_reset = -1;
+ }
+ }
if (of_property_read_u32_array(np, "ai3x-gpio-func",
ai3x_setup->gpio_func, 2) >= 0) {
--
2.15.0
^ permalink raw reply related
* [PATCH v2 5/8] ARM: dts: am335x-pepper: Fix the audio CODEC's reset pin
From: Andrew F. Davis @ 2017-11-29 17:13 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Shawn Guo, Sascha Hauer
Cc: alsa-devel, devicetree, linux-kernel, Andrew F . Davis
In-Reply-To: <20171129171359.29382-1-afd@ti.com>
The correct DT property for specifying a GPIO used for reset
is "reset-gpios", fix this here.
Fixes: 4341881d0562 ("ARM: dts: Add devicetree for Gumstix Pepper board")
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
arch/arm/boot/dts/am335x-pepper.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/am335x-pepper.dts b/arch/arm/boot/dts/am335x-pepper.dts
index 03c7d77023c6..9fb7426070ce 100644
--- a/arch/arm/boot/dts/am335x-pepper.dts
+++ b/arch/arm/boot/dts/am335x-pepper.dts
@@ -139,7 +139,7 @@
&audio_codec {
status = "okay";
- gpio-reset = <&gpio1 16 GPIO_ACTIVE_LOW>;
+ reset-gpios = <&gpio1 16 GPIO_ACTIVE_LOW>;
AVDD-supply = <&ldo3_reg>;
IOVDD-supply = <&ldo3_reg>;
DRVDD-supply = <&ldo3_reg>;
--
2.15.0
^ permalink raw reply related
* [PATCH v2 6/8] ARM: dts: imx6: RDU2: Fix the audio CODEC's reset pin
From: Andrew F. Davis @ 2017-11-29 17:13 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Shawn Guo, Sascha Hauer
Cc: alsa-devel, devicetree, linux-kernel, Andrew F . Davis
In-Reply-To: <20171129171359.29382-1-afd@ti.com>
The correct DT property for specifying a GPIO used for reset
is "reset-gpios", fix this here.
Fixes: d763762e3b58 ("ARM: dts: imx6: add ZII RDU2 boards")
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
index 7812fbac963c..1b465e5a06cf 100644
--- a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
@@ -337,7 +337,7 @@
AVDD-supply = <®_3p3v>;
IOVDD-supply = <®_3p3v>;
DVDD-supply = <&vgen4_reg>;
- gpio-reset = <&gpio1 2 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;
};
accel@1c {
@@ -525,7 +525,7 @@
AVDD-supply = <®_3p3v>;
IOVDD-supply = <®_3p3v>;
DVDD-supply = <&vgen4_reg>;
- gpio-reset = <&gpio1 0 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
};
touchscreen@20 {
--
2.15.0
^ permalink raw reply related
* [PATCH v2 7/8] ARM: dts: imx: Fix the audio CODEC's reset pin
From: Andrew F. Davis @ 2017-11-29 17:13 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Shawn Guo, Sascha Hauer
Cc: devicetree, alsa-devel, linux-kernel, Andrew F . Davis
In-Reply-To: <20171129171359.29382-1-afd@ti.com>
The correct DT property for specifying a GPIO used for reset
is "reset-gpios", fix this here.
Fixes: 50bffb78e2ee ("ARM: dts: imx: add Gateworks Ventana GW5903 support")
Signed-off-by: Andrew F. Davis <afd@ti.com>
---
arch/arm/boot/dts/imx6qdl-gw5903.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6qdl-gw5903.dtsi b/arch/arm/boot/dts/imx6qdl-gw5903.dtsi
index 444425153fc7..16299103a941 100644
--- a/arch/arm/boot/dts/imx6qdl-gw5903.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw5903.dtsi
@@ -310,7 +310,7 @@
tlv320aic3105: codec@18 {
compatible = "ti,tlv320aic3x";
reg = <0x18>;
- gpio-reset = <&gpio5 17 GPIO_ACTIVE_LOW>;
+ reset-gpios = <&gpio5 17 GPIO_ACTIVE_LOW>;
clocks = <&clks IMX6QDL_CLK_CKO>;
ai3x-micbias-vg = <2>; /* MICBIAS_2_5V */
/* Regulators */
--
2.15.0
^ permalink raw reply related
* [PATCH v2 8/8] ARM: dts: omap3-n900: Fix the audio CODEC's reset pin
From: Andrew F. Davis @ 2017-11-29 17:13 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Shawn Guo, Sascha Hauer
Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew F . Davis
In-Reply-To: <20171129171359.29382-1-afd-l0cyMroinI0@public.gmane.org>
The correct DT property for specifying a GPIO used for reset
is "reset-gpios", fix this here.
Fixes: 14e3e295b2b9 ("ARM: dts: omap3-n900: Add TLV320AIC3X support")
Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
---
arch/arm/boot/dts/omap3-n900.dts | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 669c51c00c00..5362139d5312 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -558,7 +558,7 @@
tlv320aic3x: tlv320aic3x@18 {
compatible = "ti,tlv320aic3x";
reg = <0x18>;
- gpio-reset = <&gpio2 28 GPIO_ACTIVE_HIGH>; /* 60 */
+ reset-gpios = <&gpio2 28 GPIO_ACTIVE_LOW>; /* 60 */
ai3x-gpio-func = <
0 /* AIC3X_GPIO1_FUNC_DISABLED */
5 /* AIC3X_GPIO2_FUNC_DIGITAL_MIC_INPUT */
@@ -575,7 +575,7 @@
tlv320aic3x_aux: tlv320aic3x@19 {
compatible = "ti,tlv320aic3x";
reg = <0x19>;
- gpio-reset = <&gpio2 28 GPIO_ACTIVE_HIGH>; /* 60 */
+ reset-gpios = <&gpio2 28 GPIO_ACTIVE_LOW>; /* 60 */
AVDD-supply = <&vmmc2>;
DRVDD-supply = <&vmmc2>;
--
2.15.0
--
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 related
* Re: [PATCH 3/5] PCI: cadence: Add host driver for Cadence PCIe controller
From: Lorenzo Pieralisi @ 2017-11-29 17:34 UTC (permalink / raw)
To: Cyrille Pitchen
Cc: bhelgaas, kishon, linux-pci, adouglas, stelford, dgary, kgopi,
eandrews, thomas.petazzoni, sureshp, nsekhar, linux-kernel, robh,
devicetree
In-Reply-To: <2670f7ddf59e708beb9d32bef1353e15bd4e1ecf.1511439189.git.cyrille.pitchen@free-electrons.com>
On Thu, Nov 23, 2017 at 04:01:48PM +0100, Cyrille Pitchen wrote:
> This patch adds support to the Cadence PCIe controller in host mode.
Bjorn already commented on this, it would be good to add some
of the cover letter details in this log.
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
> ---
> drivers/Makefile | 1 +
> drivers/pci/Kconfig | 1 +
> drivers/pci/cadence/Kconfig | 24 ++
> drivers/pci/cadence/Makefile | 2 +
> drivers/pci/cadence/pcie-cadence-host.c | 425 ++++++++++++++++++++++++++++++++
You should also update the MAINTAINERS file.
> drivers/pci/cadence/pcie-cadence.c | 110 +++++++++
> drivers/pci/cadence/pcie-cadence.h | 325 ++++++++++++++++++++++++
> 7 files changed, 888 insertions(+)
> create mode 100644 drivers/pci/cadence/Kconfig
> create mode 100644 drivers/pci/cadence/Makefile
> create mode 100644 drivers/pci/cadence/pcie-cadence-host.c
> create mode 100644 drivers/pci/cadence/pcie-cadence.c
> create mode 100644 drivers/pci/cadence/pcie-cadence.h
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 1d034b680431..27bdd98784d9 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -18,6 +18,7 @@ obj-y += pwm/
>
> obj-$(CONFIG_PCI) += pci/
> obj-$(CONFIG_PCI_ENDPOINT) += pci/endpoint/
> +obj-$(CONFIG_PCI_CADENCE) += pci/cadence/
Already commented on the cover letter.
> # PCI dwc controller drivers
> obj-y += pci/dwc/
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 90944667ccea..2471b2e36b8b 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -144,6 +144,7 @@ config PCI_HYPERV
> PCI devices from a PCI backend to support PCI driver domains.
>
> source "drivers/pci/hotplug/Kconfig"
> +source "drivers/pci/cadence/Kconfig"
> source "drivers/pci/dwc/Kconfig"
> source "drivers/pci/host/Kconfig"
> source "drivers/pci/endpoint/Kconfig"
> diff --git a/drivers/pci/cadence/Kconfig b/drivers/pci/cadence/Kconfig
> new file mode 100644
> index 000000000000..120306cae2aa
> --- /dev/null
> +++ b/drivers/pci/cadence/Kconfig
> @@ -0,0 +1,24 @@
> +menuconfig PCI_CADENCE
> + bool "Cadence PCI controllers support"
> + depends on PCI && HAS_IOMEM
> + help
> + Say Y here if you want to support some Cadence PCI controller.
> +
> + When in doubt, say N.
> +
> +if PCI_CADENCE
> +
> +config PCIE_CADENCE
> + bool
> +
> +config PCIE_CADENCE_HOST
> + bool "Cadence PCIe host controller"
> + depends on OF
> + depends on PCI_MSI_IRQ_DOMAIN
I do not see the reason for this dependency in the code.
> + select PCIE_CADENCE
> + help
> + Say Y here if you want to support the Cadence PCIe controller in host
> + mode. This PCIe controller may be embedded into many different vendors
> + SoCs.
> +
> +endif # PCI_CADENCE
> diff --git a/drivers/pci/cadence/Makefile b/drivers/pci/cadence/Makefile
> new file mode 100644
> index 000000000000..d57d192d2595
> --- /dev/null
> +++ b/drivers/pci/cadence/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_PCIE_CADENCE) += pcie-cadence.o
> +obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o
> diff --git a/drivers/pci/cadence/pcie-cadence-host.c b/drivers/pci/cadence/pcie-cadence-host.c
> new file mode 100644
> index 000000000000..252471e72a93
> --- /dev/null
> +++ b/drivers/pci/cadence/pcie-cadence-host.c
> @@ -0,0 +1,425 @@
> +/*
> + * Cadence PCIe host controller driver.
> + *
> + * Copyright (c) 2017 Cadence
> + *
> + * Author: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed 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/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/of_pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "pcie-cadence.h"
> +
> +/**
> + * struct cdns_pcie_rc_data - hardware specific data
> + * @max_regions: maximum number of regions supported by the hardware
> + * @vendor_id: PCI vendor ID
> + * @device_id: PCI device ID
> + * @no_bar_nbits: Number of bits to keep for inbound (PCIe -> CPU) address
> + * translation (nbits sets into the "no BAR match" register).
> + */
> +struct cdns_pcie_rc_data {
> + size_t max_regions;
Reason for it to be size_t ?
> + u16 vendor_id;
> + u16 device_id;
> + u8 no_bar_nbits;
> +};
I think that this data should come from DT (?) more below.
> +
> +/**
> + * struct cdns_pcie_rc - private data for this PCIe Root Complex driver
> + * @pcie: Cadence PCIe controller
> + * @dev: pointer to PCIe device
> + * @cfg_res: start/end offsets in the physical system memory to map PCI
> + * configuration space accesses
> + * @bus_range: first/last buses behind the PCIe host controller
> + * @cfg_base: IO mapped window to access the PCI configuration space of a
> + * single function at a time
> + * @data: pointer to a 'struct cdns_pcie_rc_data'
> + */
> +struct cdns_pcie_rc {
> + struct cdns_pcie pcie;
> + struct device *dev;
> + struct resource *cfg_res;
> + struct resource *bus_range;
> + void __iomem *cfg_base;
> + const struct cdns_pcie_rc_data *data;
> +};
> +
> +static void __iomem *
Please do not split lines like this, storage class and return type
should be in the same line as the name, move parameter(s) to a new
line.
static void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
int where)
> +cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn, int where)
> +{
> + struct pci_host_bridge *bridge = pci_find_host_bridge(bus);
> + struct cdns_pcie_rc *rc = pci_host_bridge_priv(bridge);
> + struct cdns_pcie *pcie = &rc->pcie;
> + unsigned int busn = bus->number;
> + u32 addr0, desc0;
> +
> + if (busn < rc->bus_range->start || busn > rc->bus_range->end)
> + return NULL;
It does not hurt but I wonder whether you really need this check.
> + if (busn == rc->bus_range->start) {
> + if (devfn)
I suspect I know why you need this check but I ask you to explain it
anyway if you do not mind please.
> + return NULL;
> +
> + return pcie->reg_base + (where & 0xfff);
> + }
> +
> + /* Update Output registers for AXI region 0. */
> + addr0 = CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS(12) |
Ok, so for every config access you reprogram addr0 to reflect the
correct bus|devfn ID in the PCI bus TLP corresponding to an ECAM address
in CPU physical address space, is my understanding correct ?
> + CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN(devfn) |
> + CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS(busn);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(0), addr0);
> +
> + /* Configuration Type 0 or Type 1 access. */
> + desc0 = CDNS_PCIE_AT_OB_REGION_DESC0_HARDCODED_RID |
> + CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN(0);
> + /*
> + * The bus number was already set once for all in desc1 by
> + * cdns_pcie_host_init_address_translation().
> + */
> + if (busn == rc->bus_range->start + 1)
> + desc0 |= CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE0;
> + else
> + desc0 |= CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE1;
I would like to ask you why you have to do it here and the root port
does not figure it out by itself, I do not have the datasheet so I am
just asking for my own information.
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC0(0), desc0);
> +
> + return rc->cfg_base + (where & 0xfff);
> +}
> +
> +static struct pci_ops cdns_pcie_host_ops = {
> + .map_bus = cdns_pci_map_bus,
> + .read = pci_generic_config_read,
> + .write = pci_generic_config_write,
> +};
> +
> +static const struct cdns_pcie_rc_data cdns_pcie_rc_data = {
> + .max_regions = 32,
> + .vendor_id = PCI_VENDOR_ID_CDNS,
> + .device_id = 0x0200,
> + .no_bar_nbits = 32,
> +};
Should (some of) these parameters be retrieved through a DT binding ?
> +static const struct of_device_id cdns_pcie_host_of_match[] = {
> + { .compatible = "cdns,cdns-pcie-host",
> + .data = &cdns_pcie_rc_data },
> +
> + { },
> +};
> +
> +static int cdns_pcie_parse_request_of_pci_ranges(struct device *dev,
> + struct list_head *resources,
> + struct resource **bus_range)
> +{
> + int err, res_valid = 0;
> + struct device_node *np = dev->of_node;
> + resource_size_t iobase;
> + struct resource_entry *win, *tmp;
> +
> + err = of_pci_get_host_bridge_resources(np, 0, 0xff, resources, &iobase);
> + if (err)
> + return err;
> +
> + err = devm_request_pci_bus_resources(dev, resources);
> + if (err)
> + return err;
> +
> + resource_list_for_each_entry_safe(win, tmp, resources) {
> + struct resource *res = win->res;
> +
> + switch (resource_type(res)) {
> + case IORESOURCE_IO:
> + err = pci_remap_iospace(res, iobase);
> + if (err) {
> + dev_warn(dev, "error %d: failed to map resource %pR\n",
> + err, res);
> + resource_list_destroy_entry(win);
> + }
> + break;
> + case IORESOURCE_MEM:
> + res_valid |= !(res->flags & IORESOURCE_PREFETCH);
> + break;
> + case IORESOURCE_BUS:
> + *bus_range = res;
> + break;
> + }
> + }
> +
> + if (res_valid)
> + return 0;
> +
> + dev_err(dev, "non-prefetchable memory resource required\n");
> + return -EINVAL;
Nit, I prefer you swap these two as it is done in pci-aardvark.c:
if (!res_valid) {
dev_err(dev, "non-prefetchable memory resource required\n");
return -EINVAL;
}
return 0;
but as per previous replies this function can be factorized in
core PCI code - I would not bother unless you are willing to write
the patch series that does the refactoring yourself :)
> +}
> +
> +static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
> +{
> + const struct cdns_pcie_rc_data *data = rc->data;
> + struct cdns_pcie *pcie = &rc->pcie;
> + u8 pbn, sbn, subn;
> + u32 value, ctrl;
> +
> + /*
> + * Set the root complex BAR configuration register:
> + * - disable both BAR0 and BAR1.
> + * - enable Prefetchable Memory Base and Limit registers in type 1
> + * config space (64 bits).
> + * - enable IO Base and Limit registers in type 1 config
> + * space (32 bits).
> + */
> + ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_DISABLED;
> + value = CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL(ctrl) |
> + CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL(ctrl) |
> + CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_ENABLE |
> + CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_64BITS |
> + CDNS_PCIE_LM_RC_BAR_CFG_IO_ENABLE |
> + CDNS_PCIE_LM_RC_BAR_CFG_IO_32BITS;
> + cdns_pcie_writel(pcie, CDNS_PCIE_LM_RC_BAR_CFG, value);
> +
> + /* Set root port configuration space */
> + if (data->vendor_id != 0xffff)
> + cdns_pcie_rp_writew(pcie, PCI_VENDOR_ID, data->vendor_id);
> + if (data->device_id != 0xffff)
> + cdns_pcie_rp_writew(pcie, PCI_DEVICE_ID, data->device_id);
> +
> + cdns_pcie_rp_writeb(pcie, PCI_CLASS_REVISION, 0);
> + cdns_pcie_rp_writeb(pcie, PCI_CLASS_PROG, 0);
> + cdns_pcie_rp_writew(pcie, PCI_CLASS_DEVICE, PCI_CLASS_BRIDGE_PCI);
> +
> + pbn = rc->bus_range->start;
> + sbn = pbn + 1; /* Single root port. */
> + subn = rc->bus_range->end;
> + cdns_pcie_rp_writeb(pcie, PCI_PRIMARY_BUS, pbn);
> + cdns_pcie_rp_writeb(pcie, PCI_SECONDARY_BUS, sbn);
> + cdns_pcie_rp_writeb(pcie, PCI_SUBORDINATE_BUS, subn);
Again - I do not have the datasheet for this device therefore I would
kindly ask you how this works; it seems to me that what you are doing
here is done through normal configuration cycles in an ECAM compliant
system to program the RP PRIMARY/SECONDARY/SUBORDINATE bus - I would
like to understand why this code is needed.
> + return 0;
> +}
> +
> +static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
> +{
> + struct cdns_pcie *pcie = &rc->pcie;
> + struct resource *cfg_res = rc->cfg_res;
> + struct resource *mem_res = pcie->mem_res;
> + struct resource *bus_range = rc->bus_range;
> + struct device *dev = rc->dev;
> + struct device_node *np = dev->of_node;
> + struct of_pci_range_parser parser;
> + struct of_pci_range range;
> + u32 addr0, addr1, desc1;
> + u64 cpu_addr;
> + int r, err;
> +
> + /*
> + * Reserve region 0 for PCI configure space accesses:
> + * OB_REGION_PCI_ADDR0 and OB_REGION_DESC0 are updated dynamically by
> + * cdns_pci_map_bus(), other region registers are set here once for all.
> + */
> + addr1 = 0; /* Should be programmed to zero. */
> + desc1 = CDNS_PCIE_AT_OB_REGION_DESC1_BUS(bus_range->start);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(0), addr1);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(0), desc1);
> +
> + cpu_addr = cfg_res->start - mem_res->start;
> + addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(12) |
> + (lower_32_bits(cpu_addr) & GENMASK(31, 8));
> + addr1 = upper_32_bits(cpu_addr);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(0), addr0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(0), addr1);
> +
> + err = of_pci_range_parser_init(&parser, np);
> + if (err)
> + return err;
> +
> + r = 1;
> + for_each_of_pci_range(&parser, &range) {
> + bool is_io;
> +
> + if (r >= rc->data->max_regions)
> + break;
> +
> + if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_MEM)
> + is_io = false;
> + else if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO)
> + is_io = true;
> + else
> + continue;
> +
> + cdns_pcie_set_outbound_region(pcie, r, is_io,
> + range.cpu_addr,
> + range.pci_addr,
> + range.size);
> + r++;
> + }
> +
> + /*
> + * Set Root Port no BAR match Inbound Translation registers:
> + * needed for MSI.
And DMA :) if I understand what this is doing correctly, ie setting
the root complex decoding for incoming memory traffic.
> + * Root Port BAR0 and BAR1 are disabled, hence no need to set their
> + * inbound translation registers.
> + */
> + addr0 = CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS(rc->data->no_bar_nbits);
> + addr1 = 0;
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_IB_RP_BAR_ADDR0(RP_NO_BAR), addr0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_IB_RP_BAR_ADDR1(RP_NO_BAR), addr1);
> +
> + return 0;
> +}
> +
> +static int cdns_pcie_host_init(struct device *dev,
> + struct list_head *resources,
> + struct cdns_pcie_rc *rc)
> +{
> + struct resource *bus_range = NULL;
> + int err;
> +
> + /* Parse our PCI ranges and request their resources */
> + err = cdns_pcie_parse_request_of_pci_ranges(dev, resources, &bus_range);
> + if (err)
> + goto err_out;
I think that the err_out path should be part of:
cdns_pcie_parse_request_of_pci_ranges()
implementation and here you would just return.
> +
> + if (bus_range->start > bus_range->end) {
> + err = -EINVAL;
> + goto err_out;
> + }
Add a space here; this check seems useless to me anyway.
> + rc->bus_range = bus_range;
> + rc->pcie.bus = bus_range->start;
> +
> + err = cdns_pcie_host_init_root_port(rc);
> + if (err)
> + goto err_out;
> +
> + err = cdns_pcie_host_init_address_translation(rc);
> + if (err)
> + goto err_out;
> +
> + return 0;
> +
> + err_out:
> + pci_free_resource_list(resources);
See above.
> + return err;
> +}
> +
> +static int cdns_pcie_host_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *of_id;
> + const char *type;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct pci_bus *bus, *child;
> + struct pci_host_bridge *bridge;
> + struct list_head resources;
> + struct cdns_pcie_rc *rc;
> + struct cdns_pcie *pcie;
> + struct resource *res;
> + int ret;
> +
> + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
> + if (!bridge)
> + return -ENOMEM;
> +
> + rc = pci_host_bridge_priv(bridge);
> + rc->dev = dev;
> + platform_set_drvdata(pdev, rc);
I do not think it is needed.
> + pcie = &rc->pcie;
> + pcie->is_rc = true;
> +
> + of_id = of_match_node(cdns_pcie_host_of_match, np);
> + rc->data = (const struct cdns_pcie_rc_data *)of_id->data;
> +
> + type = of_get_property(np, "device_type", NULL);
> + if (!type || strcmp(type, "pci")) {
> + dev_err(dev, "invalid \"device_type\" %s\n", type);
> + return -EINVAL;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
> + pcie->reg_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(pcie->reg_base)) {
> + dev_err(dev, "missing \"reg\"\n");
> + return PTR_ERR(pcie->reg_base);
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
> + rc->cfg_base = devm_ioremap_resource(dev, res);
devm_pci_remap_cfg_resource() please.
> + if (IS_ERR(rc->cfg_base)) {
> + dev_err(dev, "missing \"cfg\"\n");
> + return PTR_ERR(rc->cfg_base);
> + }
> + rc->cfg_res = res;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mem");
> + if (!res) {
> + dev_err(dev, "missing \"mem\"\n");
> + return -EINVAL;
> + }
> + pcie->mem_res = res;
> +
> + pm_runtime_enable(dev);
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0) {
> + dev_err(dev, "pm_runtime_get_sync() failed\n");
> + goto err_get_sync;
> + }
> +
> + INIT_LIST_HEAD(&resources);
> + ret = cdns_pcie_host_init(dev, &resources, rc);
> + if (ret)
> + goto err_init;
> +
> + list_splice_init(&resources, &bridge->windows);
> + bridge->dev.parent = dev;
> + bridge->busnr = pcie->bus;
> + bridge->ops = &cdns_pcie_host_ops;
> + bridge->map_irq = of_irq_parse_and_map_pci;
> + bridge->swizzle_irq = pci_common_swizzle;
> +
> + ret = pci_scan_root_bus_bridge(bridge);
> + if (ret < 0) {
> + dev_err(dev, "Scanning root bridge failed");
> + goto err_init;
> + }
> +
> + bus = bridge->bus;
> + pci_bus_size_bridges(bus);
> + pci_bus_assign_resources(bus);
> +
> + list_for_each_entry(child, &bus->children, node)
> + pcie_bus_configure_settings(child);
> +
> + pci_bus_add_devices(bus);
> +
> + return 0;
> +
> + err_init:
> + pm_runtime_put_sync(dev);
> +
> + err_get_sync:
> + pm_runtime_disable(dev);
> +
> + return ret;
> +}
> +
> +static struct platform_driver cdns_pcie_host_driver = {
> + .driver = {
> + .name = "cdns-pcie-host",
> + .of_match_table = cdns_pcie_host_of_match,
> + },
> + .probe = cdns_pcie_host_probe,
> +};
> +builtin_platform_driver(cdns_pcie_host_driver);
> diff --git a/drivers/pci/cadence/pcie-cadence.c b/drivers/pci/cadence/pcie-cadence.c
> new file mode 100644
> index 000000000000..5c10879d5e96
> --- /dev/null
> +++ b/drivers/pci/cadence/pcie-cadence.c
> @@ -0,0 +1,110 @@
> +/*
> + * Cadence PCIe controller driver.
> + *
> + * Copyright (c) 2017 Cadence
> + *
> + * Author: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed 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/kernel.h>
> +
> +#include "pcie-cadence.h"
> +
> +void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u32 r, bool is_io,
> + u64 cpu_addr, u64 pci_addr, size_t size)
> +{
> + /*
> + * roundup_pow_of_two() returns an unsigned long, which is not suited
> + * for 64bit values.
> + */
> + u64 sz = 1ULL << fls64(size - 1);
> + int nbits = ilog2(sz);
> + u32 addr0, addr1, desc0, desc1;
> +
> + if (nbits < 8)
> + nbits = 8;
> +
> + /* Set the PCI address */
> + addr0 = CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS(nbits) |
> + (lower_32_bits(pci_addr) & GENMASK(31, 8));
> + addr1 = upper_32_bits(pci_addr);
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(r), addr0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(r), addr1);
> +
> + /* Set the PCIe header descriptor */
> + if (is_io)
> + desc0 = CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_IO;
> + else
> + desc0 = CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_MEM;
> + desc1 = 0;
> +
> + if (pcie->is_rc) {
> + desc0 |= CDNS_PCIE_AT_OB_REGION_DESC0_HARDCODED_RID |
> + CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN(0);
> + desc1 |= CDNS_PCIE_AT_OB_REGION_DESC1_BUS(pcie->bus);
> + }
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC0(r), desc0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(r), desc1);
> +
> + /* Set the CPU address */
> + cpu_addr -= pcie->mem_res->start;
> + addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(nbits) |
> + (lower_32_bits(cpu_addr) & GENMASK(31, 8));
> + addr1 = upper_32_bits(cpu_addr);
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r), addr0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r), addr1);
> +}
> +
> +void cdns_pcie_set_outbound_region_for_normal_msg(struct cdns_pcie *pcie, u32 r,
> + u64 cpu_addr)
Not used in this patch, you should split it out.
> +{
> + u32 addr0, addr1, desc0, desc1;
> +
> + desc0 = CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_NORMAL_MSG;
> + desc1 = 0;
> + if (pcie->is_rc) {
> + desc0 |= CDNS_PCIE_AT_OB_REGION_DESC0_HARDCODED_RID |
> + CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN(0);
> + desc1 |= CDNS_PCIE_AT_OB_REGION_DESC1_BUS(pcie->bus);
> + }
> +
> + /* Set the CPU address */
> + cpu_addr -= pcie->mem_res->start;
> + addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(17) |
> + (lower_32_bits(cpu_addr) & GENMASK(31, 8));
> + addr1 = upper_32_bits(cpu_addr);
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(r), 0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(r), 0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC0(r), desc0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(r), desc1);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r), addr0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r), addr1);
> +}
> +
> +void cdns_pcie_reset_outbound_region(struct cdns_pcie *pcie, u32 r)
> +{
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(r), 0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(r), 0);
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC0(r), 0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(r), 0);
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r), 0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r), 0);
> +}
> diff --git a/drivers/pci/cadence/pcie-cadence.h b/drivers/pci/cadence/pcie-cadence.h
> new file mode 100644
> index 000000000000..195e23b7d4fe
> --- /dev/null
> +++ b/drivers/pci/cadence/pcie-cadence.h
> @@ -0,0 +1,325 @@
> +/*
> + * Cadence PCIe controller driver.
> + *
> + * Copyright (c) 2017 Cadence
> + *
> + * Author: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed 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/>.
> + */
> +
> +#ifndef _PCIE_CADENCE_H
> +#define _PCIE_CADENCE_H
> +
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +
> +/*
> + * Local Management Registers
> + */
> +#define CDNS_PCIE_LM_BASE 0x00100000
> +
> +/* Vendor ID Register */
> +#define CDNS_PCIE_LM_ID (CDNS_PCIE_LM_BASE + 0x0044)
> +#define CDNS_PCIE_LM_ID_VENDOR_MASK GENMASK(15, 0)
> +#define CDNS_PCIE_LM_ID_VENDOR_SHIFT 0
> +#define CDNS_PCIE_LM_ID_VENDOR(vid) \
> + (((vid) << CDNS_PCIE_LM_ID_VENDOR_SHIFT) & CDNS_PCIE_LM_ID_VENDOR_MASK)
> +#define CDNS_PCIE_LM_ID_SUBSYS_MASK GENMASK(31, 16)
> +#define CDNS_PCIE_LM_ID_SUBSYS_SHIFT 16
> +#define CDNS_PCIE_LM_ID_SUBSYS(sub) \
> + (((sub) << CDNS_PCIE_LM_ID_SUBSYS_SHIFT) & CDNS_PCIE_LM_ID_SUBSYS_MASK)
> +
> +/* Root Port Requestor ID Register */
> +#define CDNS_PCIE_LM_RP_RID (CDNS_PCIE_LM_BASE + 0x0228)
> +#define CDNS_PCIE_LM_RP_RID_MASK GENMASK(15, 0)
> +#define CDNS_PCIE_LM_RP_RID_SHIFT 0
> +#define CDNS_PCIE_LM_RP_RID_(rid) \
> + (((rid) << CDNS_PCIE_LM_RP_RID_SHIFT) & CDNS_PCIE_LM_RP_RID_MASK)
> +
> +/* Endpoint Bus and Device Number Register */
> +#define CDNS_PCIE_LM_EP_ID (CDNS_PCIE_LM_BASE + 0x022c)
> +#define CDNS_PCIE_LM_EP_ID_DEV_MASK GENMASK(4, 0)
> +#define CDNS_PCIE_LM_EP_ID_DEV_SHIFT 0
> +#define CDNS_PCIE_LM_EP_ID_BUS_MASK GENMASK(15, 8)
> +#define CDNS_PCIE_LM_EP_ID_BUS_SHIFT 8
> +
> +/* Endpoint Function f BAR b Configuration Registers */
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG0(fn) \
> + (CDNS_PCIE_LM_BASE + 0x0240 + (fn) * 0x0008)
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG1(fn) \
> + (CDNS_PCIE_LM_BASE + 0x0244 + (fn) * 0x0008)
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_APERTURE_MASK(b) \
> + (GENMASK(4, 0) << ((b) * 8))
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_APERTURE(b, a) \
> + (((a) << ((b) * 8)) & CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_APERTURE_MASK(b))
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL_MASK(b) \
> + (GENMASK(7, 5) << ((b) * 8))
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL(b, c) \
> + (((c) << ((b) * 8 + 5)) & CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL_MASK(b))
> +
> +/* Endpoint Function Configuration Register */
> +#define CDNS_PCIE_LM_EP_FUNC_CFG (CDNS_PCIE_LM_BASE + 0x02c0)
All endpoint defines should be moved to the patch that needs them.
> +
> +/* Root Complex BAR Configuration Register */
> +#define CDNS_PCIE_LM_RC_BAR_CFG (CDNS_PCIE_LM_BASE + 0x0300)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE_MASK GENMASK(5, 0)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE(a) \
> + (((a) << 0) & CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE_MASK)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL_MASK GENMASK(8, 6)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL(c) \
> + (((c) << 6) & CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL_MASK)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE_MASK GENMASK(13, 9)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE(a) \
> + (((a) << 9) & CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE_MASK)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL_MASK GENMASK(16, 14)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL(c) \
> + (((c) << 14) & CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL_MASK)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_ENABLE BIT(17)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_32BITS 0
> +#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_64BITS BIT(18)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_IO_ENABLE BIT(19)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_IO_16BITS 0
> +#define CDNS_PCIE_LM_RC_BAR_CFG_IO_32BITS BIT(20)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_CHECK_ENABLE BIT(31)
> +
> +/* BAR control values applicable to both Endpoint Function and Root Complex */
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_DISABLED 0x0
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_IO_32BITS 0x1
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_MEM_32BITS 0x4
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_32BITS 0x5
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_MEM_64BITS 0x6
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_64BITS 0x7
> +
> +
> +/*
> + * Endpoint Function Registers (PCI configuration space for endpoint functions)
> + */
> +#define CDNS_PCIE_EP_FUNC_BASE(fn) (((fn) << 12) & GENMASK(19, 12))
> +
> +#define CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET 0x90
> +
> +/*
> + * Root Port Registers (PCI configuration space for the root port function)
> + */
> +#define CDNS_PCIE_RP_BASE 0x00200000
> +
> +
> +/*
> + * Address Translation Registers
> + */
> +#define CDNS_PCIE_AT_BASE 0x00400000
> +
> +/* Region r Outbound AXI to PCIe Address Translation Register 0 */
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(r) \
> + (CDNS_PCIE_AT_BASE + 0x0000 + ((r) & 0x1f) * 0x0020)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS_MASK GENMASK(5, 0)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS(nbits) \
> + (((nbits) - 1) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS_MASK)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN_MASK GENMASK(19, 12)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN(devfn) \
> + (((devfn) << 12) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN_MASK)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS_MASK GENMASK(27, 20)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS(bus) \
> + (((bus) << 20) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS_MASK)
> +
> +/* Region r Outbound AXI to PCIe Address Translation Register 1 */
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(r) \
> + (CDNS_PCIE_AT_BASE + 0x0004 + ((r) & 0x1f) * 0x0020)
> +
> +/* Region r Outbound PCIe Descriptor Register 0 */
> +#define CDNS_PCIE_AT_OB_REGION_DESC0(r) \
> + (CDNS_PCIE_AT_BASE + 0x0008 + ((r) & 0x1f) * 0x0020)
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_MASK GENMASK(3, 0)
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_MEM 0x2
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_IO 0x6
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE0 0xa
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE1 0xb
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_NORMAL_MSG 0xc
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_VENDOR_MSG 0xd
> +/* Bit 23 MUST be set in RC mode. */
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_HARDCODED_RID BIT(23)
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN_MASK GENMASK(31, 24)
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN(devfn) \
> + (((devfn) << 24) & CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN_MASK)
> +
> +/* Region r Outbound PCIe Descriptor Register 1 */
> +#define CDNS_PCIE_AT_OB_REGION_DESC1(r) \
> + (CDNS_PCIE_AT_BASE + 0x000c + ((r) & 0x1f) * 0x0020)
> +#define CDNS_PCIE_AT_OB_REGION_DESC1_BUS_MASK GENMASK(7, 0)
> +#define CDNS_PCIE_AT_OB_REGION_DESC1_BUS(bus) \
> + ((bus) & CDNS_PCIE_AT_OB_REGION_DESC1_BUS_MASK)
> +
> +/* Region r AXI Region Base Address Register 0 */
> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r) \
> + (CDNS_PCIE_AT_BASE + 0x0018 + ((r) & 0x1f) * 0x0020)
> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS_MASK GENMASK(5, 0)
> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(nbits) \
> + (((nbits) - 1) & CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS_MASK)
> +
> +/* Region r AXI Region Base Address Register 1 */
> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r) \
> + (CDNS_PCIE_AT_BASE + 0x001c + ((r) & 0x1f) * 0x0020)
> +
> +/* Root Port BAR Inbound PCIe to AXI Address Translation Register */
> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0(bar) \
> + (CDNS_PCIE_AT_BASE + 0x0800 + (bar) * 0x0008)
> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS_MASK GENMASK(5, 0)
> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS(nbits) \
> + (((nbits) - 1) & CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS_MASK)
> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR1(bar) \
> + (CDNS_PCIE_AT_BASE + 0x0804 + (bar) * 0x0008)
> +
> +enum cdns_pcie_rp_bar {
> + RP_BAR0,
> + RP_BAR1,
> + RP_NO_BAR
> +};
> +
> +/* Endpoint Function BAR Inbound PCIe to AXI Address Translation Register */
> +#define CDNS_PCIE_AT_IB_EP_FUNC_BAR_ADDR0(fn, bar) \
> + (CDNS_PCIE_AT_BASE + 0x0840 + (fn) * 0x0040 + (bar) * 0x0008)
> +#define CDNS_PCIE_AT_IB_EP_FUNC_BAR_ADDR1(fn, bar) \
> + (CDNS_PCIE_AT_BASE + 0x0844 + (fn) * 0x0040 + (bar) * 0x0008)
> +
> +/* Normal/Vendor specific message access: offset inside some outbound region */
> +#define CDNS_PCIE_NORMAL_MSG_ROUTING_MASK GENMASK(7, 5)
> +#define CDNS_PCIE_NORMAL_MSG_ROUTING(route) \
> + (((route) << 5) & CDNS_PCIE_NORMAL_MSG_ROUTING_MASK)
> +#define CDNS_PCIE_NORMAL_MSG_CODE_MASK GENMASK(15, 8)
> +#define CDNS_PCIE_NORMAL_MSG_CODE(code) \
> + (((code) << 8) & CDNS_PCIE_NORMAL_MSG_CODE_MASK)
> +#define CDNS_PCIE_MSG_NO_DATA BIT(16)
> +
> +enum cdns_pcie_msg_code {
> + MSG_CODE_ASSERT_INTA = 0x20,
> + MSG_CODE_ASSERT_INTB = 0x21,
> + MSG_CODE_ASSERT_INTC = 0x22,
> + MSG_CODE_ASSERT_INTD = 0x23,
> + MSG_CODE_DEASSERT_INTA = 0x24,
> + MSG_CODE_DEASSERT_INTB = 0x25,
> + MSG_CODE_DEASSERT_INTC = 0x26,
> + MSG_CODE_DEASSERT_INTD = 0x27,
> +};
> +
> +enum cdns_pcie_msg_routing {
> + /* Route to Root Complex */
> + MSG_ROUTING_TO_RC,
> +
> + /* Use Address Routing */
> + MSG_ROUTING_BY_ADDR,
> +
> + /* Use ID Routing */
> + MSG_ROUTING_BY_ID,
> +
> + /* Route as Broadcast Message from Root Complex */
> + MSG_ROUTING_BCAST,
> +
> + /* Local message; terminate at receiver (INTx messages) */
> + MSG_ROUTING_LOCAL,
> +
> + /* Gather & route to Root Complex (PME_TO_Ack message) */
> + MSG_ROUTING_GATHER,
> +};
> +
> +/**
> + * struct cdns_pcie - private data for Cadence PCIe controller drivers
> + * @reg_base: IO mapped register base
> + * @mem_res: start/end offsets in the physical system memory to map PCI accesses
> + * @is_rc: tell whether the PCIe controller mode is Root Complex or Endpoint.
> + * @bus: In Root Complex mode, the bus number
> + */
> +struct cdns_pcie {
> + void __iomem *reg_base;
> + struct resource *mem_res;
> + bool is_rc;
> + u8 bus;
> +};
> +
> +/* Register access */
> +static inline void cdns_pcie_writeb(struct cdns_pcie *pcie, u32 reg, u8 value)
> +{
> + writeb(value, pcie->reg_base + reg);
> +}
> +
> +static inline void cdns_pcie_writew(struct cdns_pcie *pcie, u32 reg, u16 value)
> +{
> + writew(value, pcie->reg_base + reg);
> +}
> +
> +static inline void cdns_pcie_writel(struct cdns_pcie *pcie, u32 reg, u32 value)
> +{
> + writel(value, pcie->reg_base + reg);
> +}
> +
> +static inline u32 cdns_pcie_readl(struct cdns_pcie *pcie, u32 reg)
> +{
> + return readl(pcie->reg_base + reg);
> +}
> +
> +/* Root Port register access */
> +static inline void cdns_pcie_rp_writeb(struct cdns_pcie *pcie,
> + u32 reg, u8 value)
> +{
> + writeb(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
> +}
> +
> +static inline void cdns_pcie_rp_writew(struct cdns_pcie *pcie,
> + u32 reg, u16 value)
> +{
> + writew(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
> +}
> +
> +/* Endpoint Function register access */
> +static inline void cdns_pcie_ep_fn_writeb(struct cdns_pcie *pcie, u8 fn,
> + u32 reg, u8 value)
> +{
> + writeb(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
> +
> +static inline void cdns_pcie_ep_fn_writew(struct cdns_pcie *pcie, u8 fn,
> + u32 reg, u16 value)
> +{
> + writew(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
> +
> +static inline void cdns_pcie_ep_fn_writel(struct cdns_pcie *pcie, u8 fn,
> + u32 reg, u16 value)
> +{
> + writel(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
> +
> +static inline u8 cdns_pcie_ep_fn_readb(struct cdns_pcie *pcie, u8 fn, u32 reg)
> +{
> + return readb(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
> +
> +static inline u16 cdns_pcie_ep_fn_readw(struct cdns_pcie *pcie, u8 fn, u32 reg)
> +{
> + return readw(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
> +
> +static inline u32 cdns_pcie_ep_fn_readl(struct cdns_pcie *pcie, u8 fn, u32 reg)
> +{
> + return readl(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
Same comments for all endpoint related functions and defines above.
Thanks,
Lorenzo
> +void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u32 r, bool is_io,
> + u64 cpu_addr, u64 pci_addr, size_t size);
> +
> +void cdns_pcie_set_outbound_region_for_normal_msg(struct cdns_pcie *pcie, u32 r,
> + u64 cpu_addr);
> +
> +void cdns_pcie_reset_outbound_region(struct cdns_pcie *pcie, u32 r);
> +
> +#endif /* _PCIE_CADENCE_H */
> --
> 2.11.0
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply
* Re: [PATCH v2 1/2] pinctrl: Allow a device to indicate when to force a state
From: Florian Fainelli @ 2017-11-29 17:35 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-gpio, Linus Walleij, Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, ckeepax, ckeepax, swarren, andy.shevchenko, alcooperx,
bcm-kernel-feedback-list
In-Reply-To: <20171129170102.GH28152@atomide.com>
On 11/29/2017 09:01 AM, Tony Lindgren wrote:
> * Florian Fainelli <f.fainelli@gmail.com> [171102 23:18]:
>> It may happen that a device needs to force applying a state, e.g:
>> because it only defines one state of pin states (default) but loses
>> power/register contents when entering low power modes. Add a
>> pinctrl_dev::flags bitmask to help describe future quirks and define
>> PINCTRL_FLG_FORCE_STATE as such a settable flag.
>
> It makes sense to tag the existing state with the context loss
> information as otherwise we'll be duplicating the state in the
> pinctrl driver potentially for hundreds of pins.
>
> Maybe this patch description should clarify that it's the
> pinctrl device restoring the pin state, not the pinctrl
> consumer devices?
>
> So maybe just "a pinctrl device needs to force apply a state"
> instead of just device above?
It's a bit more involved than that, the pinctrl consumer device might
want to restore a particular state by calling pinctrl_select_state(),
however, because of the (p->state == state)check, the pinctrl provider
driver has no chance of making that call do the actual HW programming.
--
Florian
^ permalink raw reply
* Re: [PATCH V2 4/9] PCI: Export pci_find_host_bridge()
From: Christoph Hellwig @ 2017-11-29 17:35 UTC (permalink / raw)
To: Manikanta Maddireddy
Cc: thierry.reding, jonathanh, robh+dt, frowand.list, bhelgaas, rjw,
tglx, vidyas, kthota, linux-tegra, devicetree, linux-pci,
linux-pm
In-Reply-To: <1511638333-22951-5-git-send-email-mmaddireddy@nvidia.com>
On Sun, Nov 26, 2017 at 01:02:08AM +0530, Manikanta Maddireddy wrote:
> PCI subsystem pass pci_bus pointer to pci_ops callback functions, Tegra
> host driver use pci_find_host_bridge() to get pci_host_bridge from pci_bus.
> Export pci_find_host_bridge() to allow Tegra PCIe driver to be compiled as
> loadable kernel module.
>
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> ---
> V2:
> * commit message update
>
> drivers/pci/host-bridge.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
> index add66236215c..e0942fc086ad 100644
> --- a/drivers/pci/host-bridge.c
> +++ b/drivers/pci/host-bridge.c
> @@ -22,6 +22,7 @@ struct pci_host_bridge *pci_find_host_bridge(struct pci_bus *bus)
>
> return to_pci_host_bridge(root_bus->bridge);
> }
> +EXPORT_SYMBOL(pci_find_host_bridge);
All PCI internals should be exported using EXPORT_SYMBOL_GPL.
^ permalink raw reply
* Re: [PATCH v2 2/2] pinctrl: Allow indicating loss of pin states during low-power
From: Florian Fainelli @ 2017-11-29 17:37 UTC (permalink / raw)
To: Tony Lindgren, Linus Walleij
Cc: Florian Fainelli, linux-gpio-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, Charles Keepax, Charles Keepax, Stephen Warren,
Andy Shevchenko, Al Cooper, bcm-kernel-feedback-list
In-Reply-To: <20171129170247.GI28152-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
On 11/29/2017 09:02 AM, Tony Lindgren wrote:
> * Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [171129 13:03]:
>> On Fri, Nov 3, 2017 at 12:15 AM, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>>> Some platforms (e.g: Broadcom STB: BMIPS_GENERIC/ARCH_BRCMSTB) will lose
>>> their register contents when entering their lower power state. In such a
>>> case, the pinctrl-single driver that is used will not be able to restore
>>> the power states without telling the core about it and having
>>> pinctrl_select_state() check for that.
>>>
>>> This patch adds a new optional boolean property that Device Tree can
>>> define in order to obtain exactly that and having the core pinctrl code
>>> take that into account.
>>>
>>> Signed-off-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> Florian, I'm really sorry for losing track of this patch set, it's
>> important stuff and I see why systems are dependent on something
>> like this.
>>
>> Tony: can you look at this from a pinctrl-single point of view?
>> This is the intended consumer: pinctrl-single users that lose the
>> hardware state over suspend/resume.
>>
>> How do you see this working with other pinctrl-single users?
>
> Hmm well typically a device driver that loses it's context just does
> save and restore of the registers in runtime PM suspend/resume
> as needed. In this case it would mean duplicating the state for
> potentially for hundreds of registers.. So using the existing
> state in the pinctrl subsystem totally makes sense for the pins.
>
> Florian do you have other reasons why this should be done in the
> pinctrl framework instead of the driver? Might be worth describing
> the reasoning in the patch descriptions :)
The pinctrl provider driver that I am using is pinctrl-single, which has
proper suspend/resume callbacks but those are not causing any HW
programming to happen because of the (p->state == state) check, hence
this patch series.
>
> So as long as the pinctrl framework state is used to restore the
> state by the pinctrl driver instead of the pinctrl consumer drivers,
> I don't have issues with this patchset. So probably just improving
> the patch messages a bit should do it.
>
> FYI, on omaps, the PRCM hardware saves and restores the pinctrl
> state so this has not been so far an issue.
>
> Regards,
>
> Tony
>
>
--
Florian
--
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,01/12] hwrng: bcm2835 - Obtain base register via resource
From: Florian Fainelli @ 2017-11-29 17:38 UTC (permalink / raw)
To: Herbert Xu, Florian Fainelli
Cc: linux-kernel, Matt Mackall, Rob Herring, Mark Rutland, Ray Jui,
Scott Branden,
maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
Eric Anholt, Stefan Wahren, PrasannaKumar Muralidharan,
Russell King, Krzysztof Kozlowski, Harald Freudenberger,
Sean Wang, Martin Kaiser, Steffen Trumtrar,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE
In-Reply-To: <20171129063028.GB21594@gondor.apana.org.au>
On 11/28/2017 10:30 PM, Herbert Xu wrote:
> On Tue, Nov 07, 2017 at 04:44:38PM -0800, Florian Fainelli wrote:
>> In preparation for consolidating bcm63xx-rng into bcm2835-rng, make sure
>> that we obtain the base register via platform_get_resource() since we
>> need to support the non-DT enabled MIPS-based BCM63xx DSL SoCs.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> All applied. Thanks.
Hu, okay, I actually had a v3 prepared that I was going to post
addressing some of the comments. Should I send an incremental set of
changes now?
--
Florian
^ permalink raw reply
* Re: [PATCH v2 1/2] pinctrl: Allow a device to indicate when to force a state
From: Tony Lindgren @ 2017-11-29 17:45 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-gpio, Linus Walleij, Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, ckeepax, ckeepax, swarren, andy.shevchenko, alcooperx,
bcm-kernel-feedback-list
In-Reply-To: <d1695f25-a795-cbda-bee4-47c0b16050f8@gmail.com>
* Florian Fainelli <f.fainelli@gmail.com> [171129 17:37]:
> On 11/29/2017 09:01 AM, Tony Lindgren wrote:
> > * Florian Fainelli <f.fainelli@gmail.com> [171102 23:18]:
> >> It may happen that a device needs to force applying a state, e.g:
> >> because it only defines one state of pin states (default) but loses
> >> power/register contents when entering low power modes. Add a
> >> pinctrl_dev::flags bitmask to help describe future quirks and define
> >> PINCTRL_FLG_FORCE_STATE as such a settable flag.
> >
> > It makes sense to tag the existing state with the context loss
> > information as otherwise we'll be duplicating the state in the
> > pinctrl driver potentially for hundreds of pins.
> >
> > Maybe this patch description should clarify that it's the
> > pinctrl device restoring the pin state, not the pinctrl
> > consumer devices?
> >
> > So maybe just "a pinctrl device needs to force apply a state"
> > instead of just device above?
>
> It's a bit more involved than that, the pinctrl consumer device might
> want to restore a particular state by calling pinctrl_select_state(),
> however, because of the (p->state == state)check, the pinctrl provider
> driver has no chance of making that call do the actual HW programming.
Hmm but isn't it the pinctrl provider device losing context here?
I think the restore of the pin state should somehow happen automatically
by the pinctrl provider driver without a need for the pinctrl consumer
drivers to do anything.
Or what's the use case for pinctrl consumer driver wanting to store
a pin?
Regards,
Tony
^ permalink raw reply
* Re: [PATCH v2 1/2] ARM: dts: exynos: Switch to dedicated Odroid-XU3 sound card binding
From: Krzysztof Kozlowski @ 2017-11-29 17:55 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: devicetree, linux-samsung-soc,
Bartłomiej Żołnierkiewicz, linux-kernel, kgene,
mihailescu2m, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20171127181213.lxtbbiw75wbz2dbz@kozik-lap>
On Mon, Nov 27, 2017 at 7:12 PM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Fri, Nov 03, 2017 at 05:54:45PM +0100, Sylwester Nawrocki wrote:
>> The new sound card DT binding is used for Odroid XU3 in order
>> to properly support the HDMI audio path.
>> Clocks configuration is changed so the I2S controller is now the bit
>> and the frame clock master with EPLL as the root clock source.
>>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>> arch/arm/boot/dts/exynos4.dtsi | 1 +
>> arch/arm/boot/dts/exynos5420.dtsi | 1 +
>> arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi | 60 ++++++++++++++---------
>> 3 files changed, 40 insertions(+), 22 deletions(-)
>>
Unfortunately this patch causes the audio card to disappear on Odroid
XU3. "aplay -L" shows nothing and obviously speaker-test fails.
Applied on v4.15-rc1... any dependencies are missing?
Full boot-logs are here:
http://www.krzk.eu/#/builders/1/builds/976
(test exits on aplay -L).
Should this be dropped?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: orange-pi-zero-plus2: enable AP6212a WiFi/BT combo
From: Jagan Teki @ 2017-11-29 18:09 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Sergey Matyukevich, Maxime Ripard, Rob Herring, Mark Rutland,
devicetree, linux-arm-kernel
In-Reply-To: <CAGb2v66mNeUL0Pk60aB2s98GK2WiSdhk2_QiZJuMz0Rt=NmEyg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Nov 29, 2017 at 7:52 PM, Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> wrote:
> On Wed, Nov 29, 2017 at 9:53 PM, Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org> wrote:
>> On Sat, Nov 4, 2017 at 1:28 AM, Sergey Matyukevich <geomatsi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Enable AP6212a WiFi/BT combo chip on orange-pi-zero-plus2 board:
>>> - WiFi SDIO interface is connected to MMC1
>>> - WiFi REG_ON pin connected to gpio PA9: attach to mmc-pwrseq
>>> - WiFi HOST_WAKE pin connected to gpio PL7
>>> - BT is connected to UART1
>>>
>>> Signed-off-by: Sergey Matyukevich <geomatsi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>> .../allwinner/sun50i-h5-orangepi-zero-plus2.dts | 32 ++++++++++++++++++++++
>>> 1 file changed, 32 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts
>>> index a42fd79a62a3..d415b7b67cce 100644
>>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts
>>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts
>>> @@ -64,6 +64,13 @@
>>> regulator-min-microvolt = <3300000>;
>>> regulator-max-microvolt = <3300000>;
>>> };
>>> +
>>> + wifi_pwrseq: wifi_pwrseq {
>>> + compatible = "mmc-pwrseq-simple";
>>> + pinctrl-names = "default";
>>> + reset-gpios = <&pio 0 9 GPIO_ACTIVE_LOW>; /* PA9 */
>>> + post-power-on-delay-ms = <200>;
>>> + };
>>> };
>>>
>>> &mmc0 {
>>> @@ -75,6 +82,25 @@
>>> status = "okay";
>>> };
>>>
>>> +&mmc1 {
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&mmc1_pins_a>;
>>> + vmmc-supply = <®_vcc3v3>;
>>> + vqmmc-supply = <®_vcc3v3>;
>>> + mmc-pwrseq = <&wifi_pwrseq>;
>>> + bus-width = <4>;
>>> + non-removable;
>>> + status = "okay";
>>> +
>>> + brcmf: wifi@1 {
>>> + reg = <1>;
>>> + compatible = "brcm,bcm4329-fmac";
>>> + interrupt-parent = <&r_pio>;
>>> + interrupts = <0 7 IRQ_TYPE_LEVEL_LOW>; /* PL7 */
>>> + interrupt-names = "host-wake";
>>> + };
>>> +};
>>
>> Did you observe this issue, I'm using firmware from buildroot and
>> couldn't find any *.txt from brcm/ I believe we need ass text file for
>> firmware details, did you tried the same from BR?
>
> The text file is not provided as part of the linux-firmware repository.
> You have to fetch it from your vendor BSP. This is documented not just
> for sunxi, but also for the raspberry pi 3.
Yes, took the firmware and txt from [1] and observed the timeout.
# modprobe -a brcmfmac
[ 146.376922] cfg80211: Loading compiled-in X.509 certificates for
regulatory database
[ 146.418193] cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
[ 146.425801] platform regulatory.0: Direct firmware load for
regulatory.db failed with error -2
[ 146.434464] cfg80211: failed to load regulatory.db
# [ 146.716016] brcmfmac: brcmf_fw_map_chip_to_name: using
brcm/brcmfmac43430-sdio.bin for chip 0x00a9a6(43430) rev 0x000001
[ 147.781032] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000):
clkctl 0x50
[ 148.812968] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000):
clkctl 0x50
[ 149.824945] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000):
clkctl 0x50
[1] https://github.com/RPi-Distro/firmware-nonfree/tree/master/brcm80211/brcm
thanks!
--
Jagan Teki
Senior Linux Kernel Engineer | Amarula Solutions
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
--
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 v2 1/2] pinctrl: Allow a device to indicate when to force a state
From: Florian Fainelli @ 2017-11-29 18:15 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-gpio, Linus Walleij, Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, ckeepax, ckeepax, swarren, andy.shevchenko, alcooperx,
bcm-kernel-feedback-list
In-Reply-To: <20171129174500.GJ28152@atomide.com>
On 11/29/2017 09:45 AM, Tony Lindgren wrote:
> * Florian Fainelli <f.fainelli@gmail.com> [171129 17:37]:
>> On 11/29/2017 09:01 AM, Tony Lindgren wrote:
>>> * Florian Fainelli <f.fainelli@gmail.com> [171102 23:18]:
>>>> It may happen that a device needs to force applying a state, e.g:
>>>> because it only defines one state of pin states (default) but loses
>>>> power/register contents when entering low power modes. Add a
>>>> pinctrl_dev::flags bitmask to help describe future quirks and define
>>>> PINCTRL_FLG_FORCE_STATE as such a settable flag.
>>>
>>> It makes sense to tag the existing state with the context loss
>>> information as otherwise we'll be duplicating the state in the
>>> pinctrl driver potentially for hundreds of pins.
>>>
>>> Maybe this patch description should clarify that it's the
>>> pinctrl device restoring the pin state, not the pinctrl
>>> consumer devices?
>>>
>>> So maybe just "a pinctrl device needs to force apply a state"
>>> instead of just device above?
>>
>> It's a bit more involved than that, the pinctrl consumer device might
>> want to restore a particular state by calling pinctrl_select_state(),
>> however, because of the (p->state == state)check, the pinctrl provider
>> driver has no chance of making that call do the actual HW programming.
>
> Hmm but isn't it the pinctrl provider device losing context here?
It is the pinctrl provider indeed.
> I think the restore of the pin state should somehow happen automatically
> by the pinctrl provider driver without a need for the pinctrl consumer
> drivers to do anything.
Correct.
>
> Or what's the use case for pinctrl consumer driver wanting to store
> a pin?
I actually meant that a consumer driver could aalso call
pinctrl_select_state() in one of its resume callback for instance, but
if the pinctrl provider driver does nothing (or rather the core, on
behalf of the provider), this would be an issue. This was not super
clear, so I will stop using that example from now on :)
--
Florian
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: orange-pi-zero-plus2: enable AP6212a WiFi/BT combo
From: Sergey Matyukevich @ 2017-11-29 18:21 UTC (permalink / raw)
To: Jagan Teki
Cc: Chen-Yu Tsai, Maxime Ripard, Rob Herring, Mark Rutland,
devicetree, linux-arm-kernel
In-Reply-To: <CAMty3ZDkM7apyoJESQm0zU1t-pNFdj8ynrZJoTstvgs98Aw6ZQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Nov 29, 2017 at 11:39:35PM +0530, Jagan Teki wrote:
> On Wed, Nov 29, 2017 at 7:52 PM, Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> wrote:
> > On Wed, Nov 29, 2017 at 9:53 PM, Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org> wrote:
> >> On Sat, Nov 4, 2017 at 1:28 AM, Sergey Matyukevich <geomatsi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>> Enable AP6212a WiFi/BT combo chip on orange-pi-zero-plus2 board:
> >>> - WiFi SDIO interface is connected to MMC1
> >>> - WiFi REG_ON pin connected to gpio PA9: attach to mmc-pwrseq
> >>> - WiFi HOST_WAKE pin connected to gpio PL7
> >>> - BT is connected to UART1
> >>>
> >>> Signed-off-by: Sergey Matyukevich <geomatsi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>> ---
> >>> .../allwinner/sun50i-h5-orangepi-zero-plus2.dts | 32 ++++++++++++++++++++++
> >>> 1 file changed, 32 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts
> >>> index a42fd79a62a3..d415b7b67cce 100644
> >>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts
> >>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts
> >>> @@ -64,6 +64,13 @@
> >>> regulator-min-microvolt = <3300000>;
> >>> regulator-max-microvolt = <3300000>;
> >>> };
> >>> +
> >>> + wifi_pwrseq: wifi_pwrseq {
> >>> + compatible = "mmc-pwrseq-simple";
> >>> + pinctrl-names = "default";
> >>> + reset-gpios = <&pio 0 9 GPIO_ACTIVE_LOW>; /* PA9 */
> >>> + post-power-on-delay-ms = <200>;
> >>> + };
> >>> };
> >>>
> >>> &mmc0 {
> >>> @@ -75,6 +82,25 @@
> >>> status = "okay";
> >>> };
> >>>
> >>> +&mmc1 {
> >>> + pinctrl-names = "default";
> >>> + pinctrl-0 = <&mmc1_pins_a>;
> >>> + vmmc-supply = <®_vcc3v3>;
> >>> + vqmmc-supply = <®_vcc3v3>;
> >>> + mmc-pwrseq = <&wifi_pwrseq>;
> >>> + bus-width = <4>;
> >>> + non-removable;
> >>> + status = "okay";
> >>> +
> >>> + brcmf: wifi@1 {
> >>> + reg = <1>;
> >>> + compatible = "brcm,bcm4329-fmac";
> >>> + interrupt-parent = <&r_pio>;
> >>> + interrupts = <0 7 IRQ_TYPE_LEVEL_LOW>; /* PL7 */
> >>> + interrupt-names = "host-wake";
> >>> + };
> >>> +};
> >>
> >> Did you observe this issue, I'm using firmware from buildroot and
> >> couldn't find any *.txt from brcm/ I believe we need ass text file for
> >> firmware details, did you tried the same from BR?
> >
> > The text file is not provided as part of the linux-firmware repository.
> > You have to fetch it from your vendor BSP. This is documented not just
> > for sunxi, but also for the raspberry pi 3.
>
> Yes, took the firmware and txt from [1] and observed the timeout.
>
> # modprobe -a brcmfmac
> [ 146.376922] cfg80211: Loading compiled-in X.509 certificates for
> regulatory database
> [ 146.418193] cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
> [ 146.425801] platform regulatory.0: Direct firmware load for
> regulatory.db failed with error -2
> [ 146.434464] cfg80211: failed to load regulatory.db
> # [ 146.716016] brcmfmac: brcmf_fw_map_chip_to_name: using
> brcm/brcmfmac43430-sdio.bin for chip 0x00a9a6(43430) rev 0x000001
> [ 147.781032] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000):
> clkctl 0x50
> [ 148.812968] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000):
> clkctl 0x50
> [ 149.824945] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000):
> clkctl 0x50
>
> [1] https://github.com/RPi-Distro/firmware-nonfree/tree/master/brcm80211/brcm
There are several NVRAM file floating around. With some of them I observed the
same issue as yours. The NVRAM file from [2] worked fine with AP6212 on my
OrangePi Zero Plus2 v1.0. I haven't yet looked into NVRAM differences though...
[2] https://github.com/BPI-SINOVOIP/BPI-files/blob/master/others/brcm/lib/firmware/ap6212/nvram.txt
Regards,
Sergey
--
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 3/5] PCI: cadence: Add host driver for Cadence PCIe controller
From: Lorenzo Pieralisi @ 2017-11-29 18:25 UTC (permalink / raw)
To: Cyrille Pitchen
Cc: bhelgaas, kishon, linux-pci, adouglas, stelford, dgary, kgopi,
eandrews, thomas.petazzoni, sureshp, nsekhar, linux-kernel, robh,
devicetree
In-Reply-To: <2670f7ddf59e708beb9d32bef1353e15bd4e1ecf.1511439189.git.cyrille.pitchen@free-electrons.com>
[w/o unintended disclaimer]
On Thu, Nov 23, 2017 at 04:01:48PM +0100, Cyrille Pitchen wrote:
> This patch adds support to the Cadence PCIe controller in host mode.
Bjorn already commented on this, it would be good to add some
of the cover letter details in this log.
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
> ---
> drivers/Makefile | 1 +
> drivers/pci/Kconfig | 1 +
> drivers/pci/cadence/Kconfig | 24 ++
> drivers/pci/cadence/Makefile | 2 +
> drivers/pci/cadence/pcie-cadence-host.c | 425 ++++++++++++++++++++++++++++++++
You should also update the MAINTAINERS file.
> drivers/pci/cadence/pcie-cadence.c | 110 +++++++++
> drivers/pci/cadence/pcie-cadence.h | 325 ++++++++++++++++++++++++
> 7 files changed, 888 insertions(+)
> create mode 100644 drivers/pci/cadence/Kconfig
> create mode 100644 drivers/pci/cadence/Makefile
> create mode 100644 drivers/pci/cadence/pcie-cadence-host.c
> create mode 100644 drivers/pci/cadence/pcie-cadence.c
> create mode 100644 drivers/pci/cadence/pcie-cadence.h
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 1d034b680431..27bdd98784d9 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -18,6 +18,7 @@ obj-y += pwm/
>
> obj-$(CONFIG_PCI) += pci/
> obj-$(CONFIG_PCI_ENDPOINT) += pci/endpoint/
> +obj-$(CONFIG_PCI_CADENCE) += pci/cadence/
Already commented on the cover letter.
> # PCI dwc controller drivers
> obj-y += pci/dwc/
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 90944667ccea..2471b2e36b8b 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -144,6 +144,7 @@ config PCI_HYPERV
> PCI devices from a PCI backend to support PCI driver domains.
>
> source "drivers/pci/hotplug/Kconfig"
> +source "drivers/pci/cadence/Kconfig"
> source "drivers/pci/dwc/Kconfig"
> source "drivers/pci/host/Kconfig"
> source "drivers/pci/endpoint/Kconfig"
> diff --git a/drivers/pci/cadence/Kconfig b/drivers/pci/cadence/Kconfig
> new file mode 100644
> index 000000000000..120306cae2aa
> --- /dev/null
> +++ b/drivers/pci/cadence/Kconfig
> @@ -0,0 +1,24 @@
> +menuconfig PCI_CADENCE
> + bool "Cadence PCI controllers support"
> + depends on PCI && HAS_IOMEM
> + help
> + Say Y here if you want to support some Cadence PCI controller.
> +
> + When in doubt, say N.
> +
> +if PCI_CADENCE
> +
> +config PCIE_CADENCE
> + bool
> +
> +config PCIE_CADENCE_HOST
> + bool "Cadence PCIe host controller"
> + depends on OF
> + depends on PCI_MSI_IRQ_DOMAIN
I do not see the reason for this dependency in the code.
> + select PCIE_CADENCE
> + help
> + Say Y here if you want to support the Cadence PCIe controller in host
> + mode. This PCIe controller may be embedded into many different vendors
> + SoCs.
> +
> +endif # PCI_CADENCE
> diff --git a/drivers/pci/cadence/Makefile b/drivers/pci/cadence/Makefile
> new file mode 100644
> index 000000000000..d57d192d2595
> --- /dev/null
> +++ b/drivers/pci/cadence/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_PCIE_CADENCE) += pcie-cadence.o
> +obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host.o
> diff --git a/drivers/pci/cadence/pcie-cadence-host.c b/drivers/pci/cadence/pcie-cadence-host.c
> new file mode 100644
> index 000000000000..252471e72a93
> --- /dev/null
> +++ b/drivers/pci/cadence/pcie-cadence-host.c
> @@ -0,0 +1,425 @@
> +/*
> + * Cadence PCIe host controller driver.
> + *
> + * Copyright (c) 2017 Cadence
> + *
> + * Author: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed 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/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/of_pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "pcie-cadence.h"
> +
> +/**
> + * struct cdns_pcie_rc_data - hardware specific data
> + * @max_regions: maximum number of regions supported by the hardware
> + * @vendor_id: PCI vendor ID
> + * @device_id: PCI device ID
> + * @no_bar_nbits: Number of bits to keep for inbound (PCIe -> CPU) address
> + * translation (nbits sets into the "no BAR match" register).
> + */
> +struct cdns_pcie_rc_data {
> + size_t max_regions;
Reason for it to be size_t ?
> + u16 vendor_id;
> + u16 device_id;
> + u8 no_bar_nbits;
> +};
I think that this data should come from DT (?) more below.
> +
> +/**
> + * struct cdns_pcie_rc - private data for this PCIe Root Complex driver
> + * @pcie: Cadence PCIe controller
> + * @dev: pointer to PCIe device
> + * @cfg_res: start/end offsets in the physical system memory to map PCI
> + * configuration space accesses
> + * @bus_range: first/last buses behind the PCIe host controller
> + * @cfg_base: IO mapped window to access the PCI configuration space of a
> + * single function at a time
> + * @data: pointer to a 'struct cdns_pcie_rc_data'
> + */
> +struct cdns_pcie_rc {
> + struct cdns_pcie pcie;
> + struct device *dev;
> + struct resource *cfg_res;
> + struct resource *bus_range;
> + void __iomem *cfg_base;
> + const struct cdns_pcie_rc_data *data;
> +};
> +
> +static void __iomem *
Please do not split lines like this, storage class and return type
should be in the same line as the name, move parameter(s) to a new
line.
static void __iomem *cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn,
int where)
> +cdns_pci_map_bus(struct pci_bus *bus, unsigned int devfn, int where)
> +{
> + struct pci_host_bridge *bridge = pci_find_host_bridge(bus);
> + struct cdns_pcie_rc *rc = pci_host_bridge_priv(bridge);
> + struct cdns_pcie *pcie = &rc->pcie;
> + unsigned int busn = bus->number;
> + u32 addr0, desc0;
> +
> + if (busn < rc->bus_range->start || busn > rc->bus_range->end)
> + return NULL;
It does not hurt but I wonder whether you really need this check.
> + if (busn == rc->bus_range->start) {
> + if (devfn)
I suspect I know why you need this check but I ask you to explain it
anyway if you do not mind please.
> + return NULL;
> +
> + return pcie->reg_base + (where & 0xfff);
> + }
> +
> + /* Update Output registers for AXI region 0. */
> + addr0 = CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS(12) |
Ok, so for every config access you reprogram addr0 to reflect the
correct bus|devfn ID in the PCI bus TLP corresponding to an ECAM address
in CPU physical address space, is my understanding correct ?
> + CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN(devfn) |
> + CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS(busn);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(0), addr0);
> +
> + /* Configuration Type 0 or Type 1 access. */
> + desc0 = CDNS_PCIE_AT_OB_REGION_DESC0_HARDCODED_RID |
> + CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN(0);
> + /*
> + * The bus number was already set once for all in desc1 by
> + * cdns_pcie_host_init_address_translation().
> + */
> + if (busn == rc->bus_range->start + 1)
> + desc0 |= CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE0;
> + else
> + desc0 |= CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE1;
I would like to ask you why you have to do it here and the root port
does not figure it out by itself, I do not have the datasheet so I am
just asking for my own information.
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC0(0), desc0);
> +
> + return rc->cfg_base + (where & 0xfff);
> +}
> +
> +static struct pci_ops cdns_pcie_host_ops = {
> + .map_bus = cdns_pci_map_bus,
> + .read = pci_generic_config_read,
> + .write = pci_generic_config_write,
> +};
> +
> +static const struct cdns_pcie_rc_data cdns_pcie_rc_data = {
> + .max_regions = 32,
> + .vendor_id = PCI_VENDOR_ID_CDNS,
> + .device_id = 0x0200,
> + .no_bar_nbits = 32,
> +};
Should (some of) these parameters be retrieved through a DT binding ?
> +static const struct of_device_id cdns_pcie_host_of_match[] = {
> + { .compatible = "cdns,cdns-pcie-host",
> + .data = &cdns_pcie_rc_data },
> +
> + { },
> +};
> +
> +static int cdns_pcie_parse_request_of_pci_ranges(struct device *dev,
> + struct list_head *resources,
> + struct resource **bus_range)
> +{
> + int err, res_valid = 0;
> + struct device_node *np = dev->of_node;
> + resource_size_t iobase;
> + struct resource_entry *win, *tmp;
> +
> + err = of_pci_get_host_bridge_resources(np, 0, 0xff, resources, &iobase);
> + if (err)
> + return err;
> +
> + err = devm_request_pci_bus_resources(dev, resources);
> + if (err)
> + return err;
> +
> + resource_list_for_each_entry_safe(win, tmp, resources) {
> + struct resource *res = win->res;
> +
> + switch (resource_type(res)) {
> + case IORESOURCE_IO:
> + err = pci_remap_iospace(res, iobase);
> + if (err) {
> + dev_warn(dev, "error %d: failed to map resource %pR\n",
> + err, res);
> + resource_list_destroy_entry(win);
> + }
> + break;
> + case IORESOURCE_MEM:
> + res_valid |= !(res->flags & IORESOURCE_PREFETCH);
> + break;
> + case IORESOURCE_BUS:
> + *bus_range = res;
> + break;
> + }
> + }
> +
> + if (res_valid)
> + return 0;
> +
> + dev_err(dev, "non-prefetchable memory resource required\n");
> + return -EINVAL;
Nit, I prefer you swap these two as it is done in pci-aardvark.c:
if (!res_valid) {
dev_err(dev, "non-prefetchable memory resource required\n");
return -EINVAL;
}
return 0;
but as per previous replies this function can be factorized in
core PCI code - I would not bother unless you are willing to write
the patch series that does the refactoring yourself :)
> +}
> +
> +static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
> +{
> + const struct cdns_pcie_rc_data *data = rc->data;
> + struct cdns_pcie *pcie = &rc->pcie;
> + u8 pbn, sbn, subn;
> + u32 value, ctrl;
> +
> + /*
> + * Set the root complex BAR configuration register:
> + * - disable both BAR0 and BAR1.
> + * - enable Prefetchable Memory Base and Limit registers in type 1
> + * config space (64 bits).
> + * - enable IO Base and Limit registers in type 1 config
> + * space (32 bits).
> + */
> + ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_DISABLED;
> + value = CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL(ctrl) |
> + CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL(ctrl) |
> + CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_ENABLE |
> + CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_64BITS |
> + CDNS_PCIE_LM_RC_BAR_CFG_IO_ENABLE |
> + CDNS_PCIE_LM_RC_BAR_CFG_IO_32BITS;
> + cdns_pcie_writel(pcie, CDNS_PCIE_LM_RC_BAR_CFG, value);
> +
> + /* Set root port configuration space */
> + if (data->vendor_id != 0xffff)
> + cdns_pcie_rp_writew(pcie, PCI_VENDOR_ID, data->vendor_id);
> + if (data->device_id != 0xffff)
> + cdns_pcie_rp_writew(pcie, PCI_DEVICE_ID, data->device_id);
> +
> + cdns_pcie_rp_writeb(pcie, PCI_CLASS_REVISION, 0);
> + cdns_pcie_rp_writeb(pcie, PCI_CLASS_PROG, 0);
> + cdns_pcie_rp_writew(pcie, PCI_CLASS_DEVICE, PCI_CLASS_BRIDGE_PCI);
> +
> + pbn = rc->bus_range->start;
> + sbn = pbn + 1; /* Single root port. */
> + subn = rc->bus_range->end;
> + cdns_pcie_rp_writeb(pcie, PCI_PRIMARY_BUS, pbn);
> + cdns_pcie_rp_writeb(pcie, PCI_SECONDARY_BUS, sbn);
> + cdns_pcie_rp_writeb(pcie, PCI_SUBORDINATE_BUS, subn);
Again - I do not have the datasheet for this device therefore I would
kindly ask you how this works; it seems to me that what you are doing
here is done through normal configuration cycles in an ECAM compliant
system to program the RP PRIMARY/SECONDARY/SUBORDINATE bus - I would
like to understand why this code is needed.
> + return 0;
> +}
> +
> +static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
> +{
> + struct cdns_pcie *pcie = &rc->pcie;
> + struct resource *cfg_res = rc->cfg_res;
> + struct resource *mem_res = pcie->mem_res;
> + struct resource *bus_range = rc->bus_range;
> + struct device *dev = rc->dev;
> + struct device_node *np = dev->of_node;
> + struct of_pci_range_parser parser;
> + struct of_pci_range range;
> + u32 addr0, addr1, desc1;
> + u64 cpu_addr;
> + int r, err;
> +
> + /*
> + * Reserve region 0 for PCI configure space accesses:
> + * OB_REGION_PCI_ADDR0 and OB_REGION_DESC0 are updated dynamically by
> + * cdns_pci_map_bus(), other region registers are set here once for all.
> + */
> + addr1 = 0; /* Should be programmed to zero. */
> + desc1 = CDNS_PCIE_AT_OB_REGION_DESC1_BUS(bus_range->start);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(0), addr1);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(0), desc1);
> +
> + cpu_addr = cfg_res->start - mem_res->start;
> + addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(12) |
> + (lower_32_bits(cpu_addr) & GENMASK(31, 8));
> + addr1 = upper_32_bits(cpu_addr);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(0), addr0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(0), addr1);
> +
> + err = of_pci_range_parser_init(&parser, np);
> + if (err)
> + return err;
> +
> + r = 1;
> + for_each_of_pci_range(&parser, &range) {
> + bool is_io;
> +
> + if (r >= rc->data->max_regions)
> + break;
> +
> + if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_MEM)
> + is_io = false;
> + else if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO)
> + is_io = true;
> + else
> + continue;
> +
> + cdns_pcie_set_outbound_region(pcie, r, is_io,
> + range.cpu_addr,
> + range.pci_addr,
> + range.size);
> + r++;
> + }
> +
> + /*
> + * Set Root Port no BAR match Inbound Translation registers:
> + * needed for MSI.
And DMA :) if I understand what this is doing correctly, ie setting
the root complex decoding for incoming memory traffic.
> + * Root Port BAR0 and BAR1 are disabled, hence no need to set their
> + * inbound translation registers.
> + */
> + addr0 = CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS(rc->data->no_bar_nbits);
> + addr1 = 0;
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_IB_RP_BAR_ADDR0(RP_NO_BAR), addr0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_IB_RP_BAR_ADDR1(RP_NO_BAR), addr1);
> +
> + return 0;
> +}
> +
> +static int cdns_pcie_host_init(struct device *dev,
> + struct list_head *resources,
> + struct cdns_pcie_rc *rc)
> +{
> + struct resource *bus_range = NULL;
> + int err;
> +
> + /* Parse our PCI ranges and request their resources */
> + err = cdns_pcie_parse_request_of_pci_ranges(dev, resources, &bus_range);
> + if (err)
> + goto err_out;
I think that the err_out path should be part of:
cdns_pcie_parse_request_of_pci_ranges()
implementation and here you would just return.
> +
> + if (bus_range->start > bus_range->end) {
> + err = -EINVAL;
> + goto err_out;
> + }
Add a space here; this check seems useless to me anyway.
> + rc->bus_range = bus_range;
> + rc->pcie.bus = bus_range->start;
> +
> + err = cdns_pcie_host_init_root_port(rc);
> + if (err)
> + goto err_out;
> +
> + err = cdns_pcie_host_init_address_translation(rc);
> + if (err)
> + goto err_out;
> +
> + return 0;
> +
> + err_out:
> + pci_free_resource_list(resources);
See above.
> + return err;
> +}
> +
> +static int cdns_pcie_host_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *of_id;
> + const char *type;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct pci_bus *bus, *child;
> + struct pci_host_bridge *bridge;
> + struct list_head resources;
> + struct cdns_pcie_rc *rc;
> + struct cdns_pcie *pcie;
> + struct resource *res;
> + int ret;
> +
> + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
> + if (!bridge)
> + return -ENOMEM;
> +
> + rc = pci_host_bridge_priv(bridge);
> + rc->dev = dev;
> + platform_set_drvdata(pdev, rc);
I do not think it is needed.
> + pcie = &rc->pcie;
> + pcie->is_rc = true;
> +
> + of_id = of_match_node(cdns_pcie_host_of_match, np);
> + rc->data = (const struct cdns_pcie_rc_data *)of_id->data;
> +
> + type = of_get_property(np, "device_type", NULL);
> + if (!type || strcmp(type, "pci")) {
> + dev_err(dev, "invalid \"device_type\" %s\n", type);
> + return -EINVAL;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "reg");
> + pcie->reg_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(pcie->reg_base)) {
> + dev_err(dev, "missing \"reg\"\n");
> + return PTR_ERR(pcie->reg_base);
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
> + rc->cfg_base = devm_ioremap_resource(dev, res);
devm_pci_remap_cfg_resource() please.
> + if (IS_ERR(rc->cfg_base)) {
> + dev_err(dev, "missing \"cfg\"\n");
> + return PTR_ERR(rc->cfg_base);
> + }
> + rc->cfg_res = res;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mem");
> + if (!res) {
> + dev_err(dev, "missing \"mem\"\n");
> + return -EINVAL;
> + }
> + pcie->mem_res = res;
> +
> + pm_runtime_enable(dev);
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0) {
> + dev_err(dev, "pm_runtime_get_sync() failed\n");
> + goto err_get_sync;
> + }
> +
> + INIT_LIST_HEAD(&resources);
> + ret = cdns_pcie_host_init(dev, &resources, rc);
> + if (ret)
> + goto err_init;
> +
> + list_splice_init(&resources, &bridge->windows);
> + bridge->dev.parent = dev;
> + bridge->busnr = pcie->bus;
> + bridge->ops = &cdns_pcie_host_ops;
> + bridge->map_irq = of_irq_parse_and_map_pci;
> + bridge->swizzle_irq = pci_common_swizzle;
> +
> + ret = pci_scan_root_bus_bridge(bridge);
> + if (ret < 0) {
> + dev_err(dev, "Scanning root bridge failed");
> + goto err_init;
> + }
> +
> + bus = bridge->bus;
> + pci_bus_size_bridges(bus);
> + pci_bus_assign_resources(bus);
> +
> + list_for_each_entry(child, &bus->children, node)
> + pcie_bus_configure_settings(child);
> +
> + pci_bus_add_devices(bus);
> +
> + return 0;
> +
> + err_init:
> + pm_runtime_put_sync(dev);
> +
> + err_get_sync:
> + pm_runtime_disable(dev);
> +
> + return ret;
> +}
> +
> +static struct platform_driver cdns_pcie_host_driver = {
> + .driver = {
> + .name = "cdns-pcie-host",
> + .of_match_table = cdns_pcie_host_of_match,
> + },
> + .probe = cdns_pcie_host_probe,
> +};
> +builtin_platform_driver(cdns_pcie_host_driver);
> diff --git a/drivers/pci/cadence/pcie-cadence.c b/drivers/pci/cadence/pcie-cadence.c
> new file mode 100644
> index 000000000000..5c10879d5e96
> --- /dev/null
> +++ b/drivers/pci/cadence/pcie-cadence.c
> @@ -0,0 +1,110 @@
> +/*
> + * Cadence PCIe controller driver.
> + *
> + * Copyright (c) 2017 Cadence
> + *
> + * Author: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed 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/kernel.h>
> +
> +#include "pcie-cadence.h"
> +
> +void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u32 r, bool is_io,
> + u64 cpu_addr, u64 pci_addr, size_t size)
> +{
> + /*
> + * roundup_pow_of_two() returns an unsigned long, which is not suited
> + * for 64bit values.
> + */
> + u64 sz = 1ULL << fls64(size - 1);
> + int nbits = ilog2(sz);
> + u32 addr0, addr1, desc0, desc1;
> +
> + if (nbits < 8)
> + nbits = 8;
> +
> + /* Set the PCI address */
> + addr0 = CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS(nbits) |
> + (lower_32_bits(pci_addr) & GENMASK(31, 8));
> + addr1 = upper_32_bits(pci_addr);
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(r), addr0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(r), addr1);
> +
> + /* Set the PCIe header descriptor */
> + if (is_io)
> + desc0 = CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_IO;
> + else
> + desc0 = CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_MEM;
> + desc1 = 0;
> +
> + if (pcie->is_rc) {
> + desc0 |= CDNS_PCIE_AT_OB_REGION_DESC0_HARDCODED_RID |
> + CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN(0);
> + desc1 |= CDNS_PCIE_AT_OB_REGION_DESC1_BUS(pcie->bus);
> + }
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC0(r), desc0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(r), desc1);
> +
> + /* Set the CPU address */
> + cpu_addr -= pcie->mem_res->start;
> + addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(nbits) |
> + (lower_32_bits(cpu_addr) & GENMASK(31, 8));
> + addr1 = upper_32_bits(cpu_addr);
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r), addr0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r), addr1);
> +}
> +
> +void cdns_pcie_set_outbound_region_for_normal_msg(struct cdns_pcie *pcie, u32 r,
> + u64 cpu_addr)
Not used in this patch, you should split it out.
> +{
> + u32 addr0, addr1, desc0, desc1;
> +
> + desc0 = CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_NORMAL_MSG;
> + desc1 = 0;
> + if (pcie->is_rc) {
> + desc0 |= CDNS_PCIE_AT_OB_REGION_DESC0_HARDCODED_RID |
> + CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN(0);
> + desc1 |= CDNS_PCIE_AT_OB_REGION_DESC1_BUS(pcie->bus);
> + }
> +
> + /* Set the CPU address */
> + cpu_addr -= pcie->mem_res->start;
> + addr0 = CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(17) |
> + (lower_32_bits(cpu_addr) & GENMASK(31, 8));
> + addr1 = upper_32_bits(cpu_addr);
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(r), 0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(r), 0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC0(r), desc0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(r), desc1);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r), addr0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r), addr1);
> +}
> +
> +void cdns_pcie_reset_outbound_region(struct cdns_pcie *pcie, u32 r)
> +{
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(r), 0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(r), 0);
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC0(r), 0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_DESC1(r), 0);
> +
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r), 0);
> + cdns_pcie_writel(pcie, CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r), 0);
> +}
> diff --git a/drivers/pci/cadence/pcie-cadence.h b/drivers/pci/cadence/pcie-cadence.h
> new file mode 100644
> index 000000000000..195e23b7d4fe
> --- /dev/null
> +++ b/drivers/pci/cadence/pcie-cadence.h
> @@ -0,0 +1,325 @@
> +/*
> + * Cadence PCIe controller driver.
> + *
> + * Copyright (c) 2017 Cadence
> + *
> + * Author: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed 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/>.
> + */
> +
> +#ifndef _PCIE_CADENCE_H
> +#define _PCIE_CADENCE_H
> +
> +#include <linux/kernel.h>
> +#include <linux/pci.h>
> +
> +/*
> + * Local Management Registers
> + */
> +#define CDNS_PCIE_LM_BASE 0x00100000
> +
> +/* Vendor ID Register */
> +#define CDNS_PCIE_LM_ID (CDNS_PCIE_LM_BASE + 0x0044)
> +#define CDNS_PCIE_LM_ID_VENDOR_MASK GENMASK(15, 0)
> +#define CDNS_PCIE_LM_ID_VENDOR_SHIFT 0
> +#define CDNS_PCIE_LM_ID_VENDOR(vid) \
> + (((vid) << CDNS_PCIE_LM_ID_VENDOR_SHIFT) & CDNS_PCIE_LM_ID_VENDOR_MASK)
> +#define CDNS_PCIE_LM_ID_SUBSYS_MASK GENMASK(31, 16)
> +#define CDNS_PCIE_LM_ID_SUBSYS_SHIFT 16
> +#define CDNS_PCIE_LM_ID_SUBSYS(sub) \
> + (((sub) << CDNS_PCIE_LM_ID_SUBSYS_SHIFT) & CDNS_PCIE_LM_ID_SUBSYS_MASK)
> +
> +/* Root Port Requestor ID Register */
> +#define CDNS_PCIE_LM_RP_RID (CDNS_PCIE_LM_BASE + 0x0228)
> +#define CDNS_PCIE_LM_RP_RID_MASK GENMASK(15, 0)
> +#define CDNS_PCIE_LM_RP_RID_SHIFT 0
> +#define CDNS_PCIE_LM_RP_RID_(rid) \
> + (((rid) << CDNS_PCIE_LM_RP_RID_SHIFT) & CDNS_PCIE_LM_RP_RID_MASK)
> +
> +/* Endpoint Bus and Device Number Register */
> +#define CDNS_PCIE_LM_EP_ID (CDNS_PCIE_LM_BASE + 0x022c)
> +#define CDNS_PCIE_LM_EP_ID_DEV_MASK GENMASK(4, 0)
> +#define CDNS_PCIE_LM_EP_ID_DEV_SHIFT 0
> +#define CDNS_PCIE_LM_EP_ID_BUS_MASK GENMASK(15, 8)
> +#define CDNS_PCIE_LM_EP_ID_BUS_SHIFT 8
> +
> +/* Endpoint Function f BAR b Configuration Registers */
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG0(fn) \
> + (CDNS_PCIE_LM_BASE + 0x0240 + (fn) * 0x0008)
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG1(fn) \
> + (CDNS_PCIE_LM_BASE + 0x0244 + (fn) * 0x0008)
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_APERTURE_MASK(b) \
> + (GENMASK(4, 0) << ((b) * 8))
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_APERTURE(b, a) \
> + (((a) << ((b) * 8)) & CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_APERTURE_MASK(b))
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL_MASK(b) \
> + (GENMASK(7, 5) << ((b) * 8))
> +#define CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL(b, c) \
> + (((c) << ((b) * 8 + 5)) & CDNS_PCIE_LM_EP_FUNC_BAR_CFG_BAR_CTRL_MASK(b))
> +
> +/* Endpoint Function Configuration Register */
> +#define CDNS_PCIE_LM_EP_FUNC_CFG (CDNS_PCIE_LM_BASE + 0x02c0)
All endpoint defines should be moved to the patch that needs them.
> +
> +/* Root Complex BAR Configuration Register */
> +#define CDNS_PCIE_LM_RC_BAR_CFG (CDNS_PCIE_LM_BASE + 0x0300)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE_MASK GENMASK(5, 0)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE(a) \
> + (((a) << 0) & CDNS_PCIE_LM_RC_BAR_CFG_BAR0_APERTURE_MASK)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL_MASK GENMASK(8, 6)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL(c) \
> + (((c) << 6) & CDNS_PCIE_LM_RC_BAR_CFG_BAR0_CTRL_MASK)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE_MASK GENMASK(13, 9)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE(a) \
> + (((a) << 9) & CDNS_PCIE_LM_RC_BAR_CFG_BAR1_APERTURE_MASK)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL_MASK GENMASK(16, 14)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL(c) \
> + (((c) << 14) & CDNS_PCIE_LM_RC_BAR_CFG_BAR1_CTRL_MASK)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_ENABLE BIT(17)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_32BITS 0
> +#define CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_64BITS BIT(18)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_IO_ENABLE BIT(19)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_IO_16BITS 0
> +#define CDNS_PCIE_LM_RC_BAR_CFG_IO_32BITS BIT(20)
> +#define CDNS_PCIE_LM_RC_BAR_CFG_CHECK_ENABLE BIT(31)
> +
> +/* BAR control values applicable to both Endpoint Function and Root Complex */
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_DISABLED 0x0
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_IO_32BITS 0x1
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_MEM_32BITS 0x4
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_32BITS 0x5
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_MEM_64BITS 0x6
> +#define CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_64BITS 0x7
> +
> +
> +/*
> + * Endpoint Function Registers (PCI configuration space for endpoint functions)
> + */
> +#define CDNS_PCIE_EP_FUNC_BASE(fn) (((fn) << 12) & GENMASK(19, 12))
> +
> +#define CDNS_PCIE_EP_FUNC_MSI_CAP_OFFSET 0x90
> +
> +/*
> + * Root Port Registers (PCI configuration space for the root port function)
> + */
> +#define CDNS_PCIE_RP_BASE 0x00200000
> +
> +
> +/*
> + * Address Translation Registers
> + */
> +#define CDNS_PCIE_AT_BASE 0x00400000
> +
> +/* Region r Outbound AXI to PCIe Address Translation Register 0 */
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0(r) \
> + (CDNS_PCIE_AT_BASE + 0x0000 + ((r) & 0x1f) * 0x0020)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS_MASK GENMASK(5, 0)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS(nbits) \
> + (((nbits) - 1) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_NBITS_MASK)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN_MASK GENMASK(19, 12)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN(devfn) \
> + (((devfn) << 12) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_DEVFN_MASK)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS_MASK GENMASK(27, 20)
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS(bus) \
> + (((bus) << 20) & CDNS_PCIE_AT_OB_REGION_PCI_ADDR0_BUS_MASK)
> +
> +/* Region r Outbound AXI to PCIe Address Translation Register 1 */
> +#define CDNS_PCIE_AT_OB_REGION_PCI_ADDR1(r) \
> + (CDNS_PCIE_AT_BASE + 0x0004 + ((r) & 0x1f) * 0x0020)
> +
> +/* Region r Outbound PCIe Descriptor Register 0 */
> +#define CDNS_PCIE_AT_OB_REGION_DESC0(r) \
> + (CDNS_PCIE_AT_BASE + 0x0008 + ((r) & 0x1f) * 0x0020)
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_MASK GENMASK(3, 0)
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_MEM 0x2
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_IO 0x6
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE0 0xa
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_CONF_TYPE1 0xb
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_NORMAL_MSG 0xc
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_TYPE_VENDOR_MSG 0xd
> +/* Bit 23 MUST be set in RC mode. */
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_HARDCODED_RID BIT(23)
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN_MASK GENMASK(31, 24)
> +#define CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN(devfn) \
> + (((devfn) << 24) & CDNS_PCIE_AT_OB_REGION_DESC0_DEVFN_MASK)
> +
> +/* Region r Outbound PCIe Descriptor Register 1 */
> +#define CDNS_PCIE_AT_OB_REGION_DESC1(r) \
> + (CDNS_PCIE_AT_BASE + 0x000c + ((r) & 0x1f) * 0x0020)
> +#define CDNS_PCIE_AT_OB_REGION_DESC1_BUS_MASK GENMASK(7, 0)
> +#define CDNS_PCIE_AT_OB_REGION_DESC1_BUS(bus) \
> + ((bus) & CDNS_PCIE_AT_OB_REGION_DESC1_BUS_MASK)
> +
> +/* Region r AXI Region Base Address Register 0 */
> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0(r) \
> + (CDNS_PCIE_AT_BASE + 0x0018 + ((r) & 0x1f) * 0x0020)
> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS_MASK GENMASK(5, 0)
> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS(nbits) \
> + (((nbits) - 1) & CDNS_PCIE_AT_OB_REGION_CPU_ADDR0_NBITS_MASK)
> +
> +/* Region r AXI Region Base Address Register 1 */
> +#define CDNS_PCIE_AT_OB_REGION_CPU_ADDR1(r) \
> + (CDNS_PCIE_AT_BASE + 0x001c + ((r) & 0x1f) * 0x0020)
> +
> +/* Root Port BAR Inbound PCIe to AXI Address Translation Register */
> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0(bar) \
> + (CDNS_PCIE_AT_BASE + 0x0800 + (bar) * 0x0008)
> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS_MASK GENMASK(5, 0)
> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS(nbits) \
> + (((nbits) - 1) & CDNS_PCIE_AT_IB_RP_BAR_ADDR0_NBITS_MASK)
> +#define CDNS_PCIE_AT_IB_RP_BAR_ADDR1(bar) \
> + (CDNS_PCIE_AT_BASE + 0x0804 + (bar) * 0x0008)
> +
> +enum cdns_pcie_rp_bar {
> + RP_BAR0,
> + RP_BAR1,
> + RP_NO_BAR
> +};
> +
> +/* Endpoint Function BAR Inbound PCIe to AXI Address Translation Register */
> +#define CDNS_PCIE_AT_IB_EP_FUNC_BAR_ADDR0(fn, bar) \
> + (CDNS_PCIE_AT_BASE + 0x0840 + (fn) * 0x0040 + (bar) * 0x0008)
> +#define CDNS_PCIE_AT_IB_EP_FUNC_BAR_ADDR1(fn, bar) \
> + (CDNS_PCIE_AT_BASE + 0x0844 + (fn) * 0x0040 + (bar) * 0x0008)
> +
> +/* Normal/Vendor specific message access: offset inside some outbound region */
> +#define CDNS_PCIE_NORMAL_MSG_ROUTING_MASK GENMASK(7, 5)
> +#define CDNS_PCIE_NORMAL_MSG_ROUTING(route) \
> + (((route) << 5) & CDNS_PCIE_NORMAL_MSG_ROUTING_MASK)
> +#define CDNS_PCIE_NORMAL_MSG_CODE_MASK GENMASK(15, 8)
> +#define CDNS_PCIE_NORMAL_MSG_CODE(code) \
> + (((code) << 8) & CDNS_PCIE_NORMAL_MSG_CODE_MASK)
> +#define CDNS_PCIE_MSG_NO_DATA BIT(16)
> +
> +enum cdns_pcie_msg_code {
> + MSG_CODE_ASSERT_INTA = 0x20,
> + MSG_CODE_ASSERT_INTB = 0x21,
> + MSG_CODE_ASSERT_INTC = 0x22,
> + MSG_CODE_ASSERT_INTD = 0x23,
> + MSG_CODE_DEASSERT_INTA = 0x24,
> + MSG_CODE_DEASSERT_INTB = 0x25,
> + MSG_CODE_DEASSERT_INTC = 0x26,
> + MSG_CODE_DEASSERT_INTD = 0x27,
> +};
> +
> +enum cdns_pcie_msg_routing {
> + /* Route to Root Complex */
> + MSG_ROUTING_TO_RC,
> +
> + /* Use Address Routing */
> + MSG_ROUTING_BY_ADDR,
> +
> + /* Use ID Routing */
> + MSG_ROUTING_BY_ID,
> +
> + /* Route as Broadcast Message from Root Complex */
> + MSG_ROUTING_BCAST,
> +
> + /* Local message; terminate at receiver (INTx messages) */
> + MSG_ROUTING_LOCAL,
> +
> + /* Gather & route to Root Complex (PME_TO_Ack message) */
> + MSG_ROUTING_GATHER,
> +};
> +
> +/**
> + * struct cdns_pcie - private data for Cadence PCIe controller drivers
> + * @reg_base: IO mapped register base
> + * @mem_res: start/end offsets in the physical system memory to map PCI accesses
> + * @is_rc: tell whether the PCIe controller mode is Root Complex or Endpoint.
> + * @bus: In Root Complex mode, the bus number
> + */
> +struct cdns_pcie {
> + void __iomem *reg_base;
> + struct resource *mem_res;
> + bool is_rc;
> + u8 bus;
> +};
> +
> +/* Register access */
> +static inline void cdns_pcie_writeb(struct cdns_pcie *pcie, u32 reg, u8 value)
> +{
> + writeb(value, pcie->reg_base + reg);
> +}
> +
> +static inline void cdns_pcie_writew(struct cdns_pcie *pcie, u32 reg, u16 value)
> +{
> + writew(value, pcie->reg_base + reg);
> +}
> +
> +static inline void cdns_pcie_writel(struct cdns_pcie *pcie, u32 reg, u32 value)
> +{
> + writel(value, pcie->reg_base + reg);
> +}
> +
> +static inline u32 cdns_pcie_readl(struct cdns_pcie *pcie, u32 reg)
> +{
> + return readl(pcie->reg_base + reg);
> +}
> +
> +/* Root Port register access */
> +static inline void cdns_pcie_rp_writeb(struct cdns_pcie *pcie,
> + u32 reg, u8 value)
> +{
> + writeb(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
> +}
> +
> +static inline void cdns_pcie_rp_writew(struct cdns_pcie *pcie,
> + u32 reg, u16 value)
> +{
> + writew(value, pcie->reg_base + CDNS_PCIE_RP_BASE + reg);
> +}
> +
> +/* Endpoint Function register access */
> +static inline void cdns_pcie_ep_fn_writeb(struct cdns_pcie *pcie, u8 fn,
> + u32 reg, u8 value)
> +{
> + writeb(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
> +
> +static inline void cdns_pcie_ep_fn_writew(struct cdns_pcie *pcie, u8 fn,
> + u32 reg, u16 value)
> +{
> + writew(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
> +
> +static inline void cdns_pcie_ep_fn_writel(struct cdns_pcie *pcie, u8 fn,
> + u32 reg, u16 value)
> +{
> + writel(value, pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
> +
> +static inline u8 cdns_pcie_ep_fn_readb(struct cdns_pcie *pcie, u8 fn, u32 reg)
> +{
> + return readb(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
> +
> +static inline u16 cdns_pcie_ep_fn_readw(struct cdns_pcie *pcie, u8 fn, u32 reg)
> +{
> + return readw(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
> +
> +static inline u32 cdns_pcie_ep_fn_readl(struct cdns_pcie *pcie, u8 fn, u32 reg)
> +{
> + return readl(pcie->reg_base + CDNS_PCIE_EP_FUNC_BASE(fn) + reg);
> +}
Same comments for all endpoint related functions and defines above.
Thanks,
Lorenzo
> +void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u32 r, bool is_io,
> + u64 cpu_addr, u64 pci_addr, size_t size);
> +
> +void cdns_pcie_set_outbound_region_for_normal_msg(struct cdns_pcie *pcie, u32 r,
> + u64 cpu_addr);
> +
> +void cdns_pcie_reset_outbound_region(struct cdns_pcie *pcie, u32 r);
> +
> +#endif /* _PCIE_CADENCE_H */
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH v2 1/2] pinctrl: Allow a device to indicate when to force a state
From: Tony Lindgren @ 2017-11-29 18:27 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-gpio, Linus Walleij, Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, ckeepax, ckeepax, swarren, andy.shevchenko, alcooperx,
bcm-kernel-feedback-list
In-Reply-To: <ff2e6268-4738-4858-5cf0-368a23c81ccc@gmail.com>
* Florian Fainelli <f.fainelli@gmail.com> [171129 18:17]:
> On 11/29/2017 09:45 AM, Tony Lindgren wrote:
> > * Florian Fainelli <f.fainelli@gmail.com> [171129 17:37]:
> >> On 11/29/2017 09:01 AM, Tony Lindgren wrote:
> >>> * Florian Fainelli <f.fainelli@gmail.com> [171102 23:18]:
> >>>> It may happen that a device needs to force applying a state, e.g:
> >>>> because it only defines one state of pin states (default) but loses
> >>>> power/register contents when entering low power modes. Add a
> >>>> pinctrl_dev::flags bitmask to help describe future quirks and define
> >>>> PINCTRL_FLG_FORCE_STATE as such a settable flag.
> >>>
> >>> It makes sense to tag the existing state with the context loss
> >>> information as otherwise we'll be duplicating the state in the
> >>> pinctrl driver potentially for hundreds of pins.
> >>>
> >>> Maybe this patch description should clarify that it's the
> >>> pinctrl device restoring the pin state, not the pinctrl
> >>> consumer devices?
> >>>
> >>> So maybe just "a pinctrl device needs to force apply a state"
> >>> instead of just device above?
> >>
> >> It's a bit more involved than that, the pinctrl consumer device might
> >> want to restore a particular state by calling pinctrl_select_state(),
> >> however, because of the (p->state == state)check, the pinctrl provider
> >> driver has no chance of making that call do the actual HW programming.
> >
> > Hmm but isn't it the pinctrl provider device losing context here?
>
> It is the pinctrl provider indeed.
>
> > I think the restore of the pin state should somehow happen automatically
> > by the pinctrl provider driver without a need for the pinctrl consumer
> > drivers to do anything.
>
> Correct.
OK thanks for confirming that.
> > Or what's the use case for pinctrl consumer driver wanting to store
> > a pin?
>
> I actually meant that a consumer driver could aalso call
> pinctrl_select_state() in one of its resume callback for instance, but
> if the pinctrl provider driver does nothing (or rather the core, on
> behalf of the provider), this would be an issue. This was not super
> clear, so I will stop using that example from now on :)
OK yeah that's probably where the confusion comes from :)
Tony
^ 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