Devicetree
 help / color / mirror / Atom feed
* [PATCH 1/2] devicetree: Add devicetree bindings documentation for the ADAU1977
From: Lars-Peter Clausen @ 2014-02-03 14:57 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Lars-Peter Clausen,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA

The ADAU1977 is a four channel audio ADC. It can either be connected to an I2C
or a SPI bus. Both bindings are described in this document. Other than the
standard I2C and SPI bus properties a regulator for the AVDD supply needs to be
specified in the bindings. Optionally, if present in the hardware design, a
regulator for the DVDD supply and a GPIO connected to the chips reset pin can be
specified. The bindings also allow to specify the microphone bias voltage that
should be used with the hardware design.

Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
---
 .../devicetree/bindings/sound/adi,adau1977.txt     | 58 ++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/adi,adau1977.txt

diff --git a/Documentation/devicetree/bindings/sound/adi,adau1977.txt b/Documentation/devicetree/bindings/sound/adi,adau1977.txt
new file mode 100644
index 0000000..3cf03d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/adi,adau1977.txt
@@ -0,0 +1,58 @@
+Analog Devices ADAU1977, ADAU1978, ADAU1979 devicetree bindings
+
+The ADAU1977 is a four channel audio ADC. It can either be connected to an I2C
+or a SPI bus. Both bindings are described in this document. Depending on the bus
+type different properties must/can be specified.
+
+Shared required properties:
+ - compatible: Must be "adi,adau1977", "adi,adau1978" or adi,adau1979"
+ - AVDD-supply: A handle to the power supply connected to the AVDD pin
+
+Shared optional properties:
+ - DVDD-supply: A handle to the power supply connected to the DVDD pin. If
+   specified the internal LDO will be disabled.
+ - reset-gpios: GPIO chip handle and specifier that define which GPIO is
+   connected to the chips PD/RST pin. If not specified the reset pin is assumed
+   to be hardwired to VCC.
+ - adi,micbias: Configures the voltage setting for the MICBIAS pin. If not
+   specified the default value will be 8.5 Volts. This property is only valid
+   for the ADAU1977.
+   Valid values for this property are:
+	0: 5.0V
+	1: 5.5V
+	2: 6.0V
+	3: 6.5V
+	4: 7.0V
+	5: 7.5V
+	6: 8.0V
+	7: 8.5V
+	8: 9.0V
+
+I2C required properties:
+ - reg: The I2C address of the chip
+
+SPI required properties:
+ - reg: The SPI chipselect signal of the SPI master associated with this chip
+ - spi-max-frequency: Maximum spi frequency to use.
+
+Examples:
+
+&i2c_bus {
+	adau1977: codec@11 {
+		compatible = "adi,adau1977";
+		reg = <0x11>;
+		reset-gpios = <&gpio 5 0>;
+		AVDD-supply = <&codec_avdd_supply>;
+		DVDD-supply = <&codec_dvdd_supply>;
+	};
+};
+
+&spi_bus {
+	adau1977: codec@1 {
+		compatible = "adi,adau1977";
+		reg = <0x1>;
+		spi-max-frequency = <10000000>;
+		AVDD-supply = <&codec_avdd_supply>;
+		adi,micbias = <5>;
+	};
+};
-- 
1.8.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 09/10] watchdog: xilinx: Add missing binding
From: Arnd Bergmann @ 2014-02-03 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Michal Simek, linux-kernel, monstr, Mark Rutland, devicetree,
	Pawel Moll, Ian Campbell, linux-doc, Rob Herring, Rob Landley,
	Kumar Gala
In-Reply-To: <9c721b89ed2ceb6997809bb3363f852277e67dc2.1391177880.git.michal.simek@xilinx.com>

On Friday 31 January 2014, Michal Simek wrote:
> +Optional properties:
> +- clock-frequency      : Frequency of clock in Hz
> +- xlnx,wdt-enable-once : 0 - Watchdog can be restarted
> +                         1 - Watchdog can be enabled just once
> +- xlnx,wdt-interval    : Watchdog timeout interval in 2^<val> clock cycles,
> +                         <val> is integer from 8 to 31.
> +

The latter two don't really seem to be xilinx specific, it would be
reasonable to have a standard watchdog binding that mandates a common
format for them.

I'm not sure about the enable-once flag, which seems to just map to the
"nowayout" watchdog option that is not a hardware feature at all
and should probably be kept as a software setting only, rather than
settable through DT. If it is kept, it should have a standard name and
get turned into a boolean (present/absent) property rather than a
0/1 integer property.

The interval should really be specified in terms of seconds or miliseconds,
not in clock cycles.

	Arnd

^ permalink raw reply

* Re: [PATCH] of: add vendor prefix for Honeywell
From: Olof Johansson @ 2014-02-03 15:12 UTC (permalink / raw)
  To: Rob Herring, Belisko Marek, NeilBrown
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Rob Landley, devicetree@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq++pxMmJrn+=dreFJMCT2EUxbOuhrGetBecUTb2ax4YOw@mail.gmail.com>



On February 3, 2014 6:46:58 AM PST, Rob Herring <robherring2@gmail.com> wrote:
>On Fri, Jan 31, 2014 at 3:46 PM, Belisko Marek
><marek.belisko@gmail.com> wrote:
>> On Fri, Jan 31, 2014 at 7:53 PM, Olof Johansson <olof@lixom.net>
>wrote:
>>> On Sat, Jan 25, 2014 at 1:38 PM, Marek Belisko <marek@goldelico.com>
>wrote:
>>>> Signed-off-by: Marek Belisko <marek@goldelico.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt
>b/Documentation/devicetree/bindings/vendor-prefixes.txt
>>>> index 3f900cd..b14c617 100644
>>>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>>>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>>>> @@ -40,6 +40,7 @@ gmt   Global Mixed-mode Technology, Inc.
>>>>  gumstix        Gumstix, Inc.
>>>>  haoyu  Haoyu Microelectronic Co. Ltd.
>>>>  hisilicon      Hisilicon Limited.
>>>> +honeywell      Honeywell
>>>
>>> The norm has usually been to use the the stock ticker symbol of a
>>> public company if it is available. In other words, I think the
>prefix
>>> here should be 'hon', since Honeywell is a publicly traded company
>>> under that ticker.
>> OK I didn't know about that. I just pick name from here:
>> Documentation/devicetree/bindings/iio/magnetometer/hmc5843.txt
>> So we need to update also compatible property in mentioned file.
>
>It appears this is already used in omap4-sdp dts for some time, so I'm
>inclined to leave it as honeywell to avoid breaking old dtbs.

Fair enough.

It would be good to find a way to avoid too much ambiguity around prefixes longer term but that's a discussion for another day.

-Olof


-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply

* Re: [PATCH 09/10] watchdog: xilinx: Add missing binding
From: Michal Simek @ 2014-02-03 15:13 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, Michal Simek, linux-kernel, monstr,
	Mark Rutland, devicetree, Pawel Moll, Ian Campbell, linux-doc,
	Rob Herring, Rob Landley, Kumar Gala
In-Reply-To: <201402031606.11753.arnd@arndb.de>

On 02/03/2014 04:06 PM, Arnd Bergmann wrote:
> On Friday 31 January 2014, Michal Simek wrote:
>> +Optional properties:
>> +- clock-frequency      : Frequency of clock in Hz
>> +- xlnx,wdt-enable-once : 0 - Watchdog can be restarted
>> +                         1 - Watchdog can be enabled just once
>> +- xlnx,wdt-interval    : Watchdog timeout interval in 2^<val> clock cycles,
>> +                         <val> is integer from 8 to 31.
>> +
> 
> The latter two don't really seem to be xilinx specific, it would be
> reasonable to have a standard watchdog binding that mandates a common
> format for them.
> 
> I'm not sure about the enable-once flag, which seems to just map to the
> "nowayout" watchdog option that is not a hardware feature at all
> and should probably be kept as a software setting only, rather than
> settable through DT. If it is kept, it should have a standard name and
> get turned into a boolean (present/absent) property rather than a
> 0/1 integer property.
> 
> The interval should really be specified in terms of seconds or miliseconds,
> not in clock cycles.

Intention wasn't to fix binding but document current one
which is in mainline for a long time.

Apart of this - yes, wdt-enable-once is nowayout and wdt-interval should be timeout
is seconds, and clock-frequency should go out and use CCF for getting clock.

Thanks,
Michal



^ permalink raw reply

* Re: [PATCH 1/2] devicetree: Add devicetree bindings documentation for the ADAU1977
From: Geert Uytterhoeven @ 2014-02-03 15:15 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Mark Brown, Liam Girdwood, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1391439428-3198-1-git-send-email-lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>

On Mon, Feb 3, 2014 at 3:57 PM, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> wrote:
> + - adi,micbias: Configures the voltage setting for the MICBIAS pin. If not
> +   specified the default value will be 8.5 Volts. This property is only valid
> +   for the ADAU1977.
> +   Valid values for this property are:
> +       0: 5.0V
> +       1: 5.5V
> +       2: 6.0V
> +       3: 6.5V
> +       4: 7.0V
> +       5: 7.5V
> +       6: 8.0V
> +       7: 8.5V
> +       8: 9.0V

This means the casual DTS reader need to know about the meaning of
the values. You can avoid that by:
  1. Using preprocessor defines. Drawback is that you have to add an
     include file.
  2. Using mV, and specify minimum, maximum, and incrementals in the
     bindings, cfr.
Documentation/devicetree/bindings/power_supply/ti,bq24735.txt
     Drawback is that the user can easily specify invalid values.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] spi: rspi: fix build error when CONFIG_OF is not set
From: Mark Brown @ 2014-02-03 15:17 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Shimoda, Yoshihiro, Grant Likely, Rob Herring,
	linux-spi-u79uwXL29TY76Z2rM5mHXA, SH-Linux,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Development
In-Reply-To: <alpine.DEB.2.02.1402030939520.25794-97SZ98TBZzA1xEWliksxXw@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 696 bytes --]

On Mon, Feb 03, 2014 at 09:47:25AM +0100, Geert Uytterhoeven wrote:

> Thanks, obviously I missed that of_match_device() still uses the ID table
> parameter if CONFIG_OF=n :-(

> Below I have two alternative solutions:
>   1. Uses rspi_of_match() to nullify the ID table pointer, like is done in
>      the platform_driver structure,
>   2. Fixes it at the OF subsystem level, by nullifying the ID table pointer
>      inside of_match_device().

> If 2 is accepted, drivers don't have to care about this anymore.

I think if we can fix it at the subsystem level that's clearly nicer
since it means nothing else will run into the same issue.  Your patch
looks OK to me, I'd suggest submitting it.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] of: fix PCI bus match for PCIe slots
From: Kleber Sacilotto de Souza @ 2014-02-03 15:20 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Benjamin Herrenschmidt, Brian King, Grant Likely, Rob Herring
In-Reply-To: <CAL_JsqKUFBSchVZivQ-qsaX4_UtC4fKYa8-Qw2o1zJMZTyk6Fg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 02/01/2014 08:54 PM, Rob Herring wrote:
> On Thu, Jan 30, 2014 at 5:29 PM, Kleber Sacilotto de Souza
> <klebers-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
>> On IBM pseries systems the device_type device-tree property of a PCIe
>> bridge contains the string "pciex". The of_bus_pci_match() function was
>> looking only for "pci" on this property, so in such cases the bus
>> matching code was falling back to the default bus, causing problems on
>> functions that should be using "assigned-addresses" for region address
>> translation. This patch fixes the problem by also looking for "pciex" on
>> the PCI bus match function.
>
> Does this need to go to stable?

Yes, we need this fix on older kernels as well.

>
>> Signed-off-by: Kleber Sacilotto de Souza <klebers-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> ---
>>   drivers/of/address.c |    4 ++--
>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/of/address.c b/drivers/of/address.c
>> index d3dd41c..0da2c0f 100644
>> --- a/drivers/of/address.c
>> +++ b/drivers/of/address.c
>> @@ -102,8 +102,8 @@ static int of_bus_pci_match(struct device_node *np)
>>           * "vci" is for the /chaos bridge on 1st-gen PCI powermacs
>>           * "ht" is hypertransport
>
> How about a comment for who needs pciex.

Ok, I will add a comment and send a v2.


Thanks,

-- 
Kleber Sacilotto de Souza
IBM Linux Technology Center

--
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] of: fix PCI bus match for PCIe slots
From: Kleber Sacilotto de Souza @ 2014-02-03 15:31 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Benjamin Herrenschmidt, Brian King, Grant Likely, Rob Herring,
	Kleber Sacilotto de Souza

On IBM pseries systems the device_type device-tree property of a PCIe
bridge contains the string "pciex". The of_bus_pci_match() function was
looking only for "pci" on this property, so in such cases the bus
matching code was falling back to the default bus, causing problems on
functions that should be using "assigned-addresses" for region address
translation. This patch fixes the problem by also looking for "pciex" on
the PCI bus match function.

v2: added comment

Signed-off-by: Kleber Sacilotto de Souza <klebers-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 drivers/of/address.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index d3dd41c..1a54f1f 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -99,11 +99,12 @@ static unsigned int of_bus_default_get_flags(const __be32 *addr)
 static int of_bus_pci_match(struct device_node *np)
 {
 	/*
+ 	 * "pciex" is PCI Express
 	 * "vci" is for the /chaos bridge on 1st-gen PCI powermacs
 	 * "ht" is hypertransport
 	 */
-	return !strcmp(np->type, "pci") || !strcmp(np->type, "vci") ||
-		!strcmp(np->type, "ht");
+	return !strcmp(np->type, "pci") || !strcmp(np->type, "pciex") ||
+		!strcmp(np->type, "vci") || !strcmp(np->type, "ht");
 }
 
 static void of_bus_pci_count_cells(struct device_node *np,
-- 
1.7.1

--
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 09/10] watchdog: xilinx: Add missing binding
From: Arnd Bergmann @ 2014-02-03 15:32 UTC (permalink / raw)
  To: Michal Simek
  Cc: linux-arm-kernel, linux-kernel, monstr, Mark Rutland, devicetree,
	Pawel Moll, Ian Campbell, linux-doc, Rob Herring, Rob Landley,
	Kumar Gala
In-Reply-To: <24ecacca-2714-4834-921c-b827c0e592a3@VA3EHSMHS003.ehs.local>

On Monday 03 February 2014 16:13:47 Michal Simek wrote:
> Intention wasn't to fix binding but document current one
> which is in mainline for a long time.

Ok, I see.

> Apart of this - yes, wdt-enable-once is nowayout and wdt-interval should be timeout
> is seconds, and clock-frequency should go out and use CCF for getting clock.

Could we make a common binding then, and document that the xilinx
watchdog can optionally provide either one?

	Arnd

^ permalink raw reply

* Re: [PATCH 09/10] watchdog: xilinx: Add missing binding
From: Michal Simek @ 2014-02-03 15:47 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Michal Simek, linux-arm-kernel, linux-kernel, monstr,
	Mark Rutland, devicetree, Pawel Moll, Ian Campbell, linux-doc,
	Rob Herring, Rob Landley, Kumar Gala
In-Reply-To: <3121595.GbLMuGPlvA@wuerfel>

On 02/03/2014 04:32 PM, Arnd Bergmann wrote:
> On Monday 03 February 2014 16:13:47 Michal Simek wrote:
>> Intention wasn't to fix binding but document current one
>> which is in mainline for a long time.
> 
> Ok, I see.
> 
>> Apart of this - yes, wdt-enable-once is nowayout and wdt-interval should be timeout
>> is seconds, and clock-frequency should go out and use CCF for getting clock.
> 
> Could we make a common binding then, and document that the xilinx
> watchdog can optionally provide either one?

Do you mean to have 2 DT bindings?

This binding is used from 2011-07.
It means it was generated for all hw designs at least from this time.
I would say from DT usage on Microblaze because it is not special case
in our dt generator.

xlnx,XXX are XXX parameters which you have to setup in tools
and get synthesized. This is valid for all xilinx IPs. We have full
IP description by generating xlnx,XXX parameters directly from tools
because we know all variants which can happen.

Just back to your previous post:
"I'm not sure about the enable-once flag, which seems to just map to the
"nowayout" watchdog option that is not a hardware feature at all"
this is hw feature which you can select in tools because this is fpga. :-)

Thanks,
Michal



^ permalink raw reply

* Re: [PATCH v5 00/14] ahci: library-ise ahci_platform, add sunxi driver and cleanup imx driver
From: Tejun Heo @ 2014-02-03 16:09 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Oliver Schinagl, Maxime Ripard, Richard Zhu, Roger Quadros,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <1390417489-5354-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Hello,

On Wed, Jan 22, 2014 at 08:04:35PM +0100, Hans de Goede wrote:
> Here is v4 of my patchset for adding ahci-sunxi support. It has grown quite
> a bit since I've been going for a more generic approach this time and I've
> also cleaned up the ahci-imx driver to use the same generic approach.

On top of which tree were these patches generated?  They don't apply
to 3.14-rc1.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH RFC v4 00/10] ahci: library-ise ahci_platform, add sunxi driver and cleanup imx driver
From: Rob Herring @ 2014-02-03 16:31 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Tejun Heo, devicetree, linux-ide-u79uwXL29TY76Z2rM5mHXA,
	Oliver Schinagl, Richard Zhu, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	Maxime Ripard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Roger Quadros
In-Reply-To: <1390236303-22538-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Mon, Jan 20, 2014 at 10:44 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Hi All,
>
> Here is v4 of my patchset for adding ahci-sunxi support. It has grown quite
> a bit since I've been going for a more generic approach this time and I've
> also cleaned up the ahci-imx driver to use the same generic approach.

I don't know if you missed it or just don't intend to refactor all
AHCI drivers, but the highbank driver could benefit from this as well.

Rob

>
> History:
>
> v1, by Olliver Schinagl:
> This was using the approach of having a platform device which probe method
> creates a new child platform device which gets driven by ahci_platform.c,
> as done by ahci_imx.c .
>
> v2, by Hans de Goede:
> Stand-alone platform driver based on Olliver's work
>
> v3, by Hans de Goede:
> patch-series, with 4 different parts
> a) Make ahci_platform.c more generic, handle more then 1 clk, target pwr
>    regulator
> b) New ahci-sunxi code only populating ahci_platform_data, passed to
>    ahci_platform.c to of_device_id matching.
> c) Refactor ahci-imx code to work the same as the new ahci-sunxi code, this
>    is the reason why v3 is an RFC, I'm waiting for the wandboard I ordered to
>    arrive so that I can actually test this.
> d) dts bindings for the sunxi ahci parts
>
> v4, by Hans de Goede:
> patch-series, with 5 different parts:
> a) Make ahci_platform.c more generic, handle more then 1 clk, target pwr
>    regulator
> b) Turn parts of ahci_platform.c into a library for use by other drivers
> c) New ahci-sunxi driver using the ahci_platform.c library functionality
> d) Refactor ahci-imx code to work the same as the new ahci-sunxi code
> e) dts bindings for the sunxi ahci parts
>
> Parts a-d are intended for merging through the ata tree, the dts bindings
> will be merged to Maxime Ripard's sunxi-dts tree.
>
> Regards,
>
> Hans
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 08/11] of: Increase MAX_PHANDLE_ARGS
From: Will Deacon @ 2014-02-03 16:44 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	Rob Herring, Andreas Herrmann,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CAL_Jsq+=dm4kPk=e0h_up9=wvED4fd3MBtSNFxm2NEz_yag-uA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Jan 31, 2014 at 04:24:09PM +0000, Rob Herring wrote:
> On Thu, Jan 30, 2014 at 11:45 AM, Andreas Herrmann
> <andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org> wrote:
> > Do you agree on increasing MAX_PHANDLE_ARGS to 32?
> 
> Yes, but more than that will require a closer look. Please get this
> into next early in the cycle.

If you want to get this patch in early, I suggest taking it via a tree other
than mine. I don't put my iommu queue into -next, since it goes to Joerg
around -rc4, who then sticks it in after that.

Will

^ permalink raw reply

* Re: [PATCH v1] ARM: tegra: add nvidia,wdt-timer-id optional property
From: Mark Rutland @ 2014-02-03 17:10 UTC (permalink / raw)
  To: Andrew Chew
  Cc: robh+dt@kernel.org, Pawel Moll, ijc+devicetree@hellion.org.uk,
	galak@codeaurora.org, rob@landley.net, swarren@wwwdotorg.org,
	thierry.reding@gmail.com, abrestic@chromium.org,
	dgreid@chromium.org, katierh@chromium.org,
	devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1391204811-6293-1-git-send-email-achew@nvidia.com>

On Fri, Jan 31, 2014 at 09:46:51PM +0000, Andrew Chew wrote:
> This optional property can be used to specify which timers are to be used
> for hardware watchdog timeouts (via a tegra wdt driver).

Is there any reason that a particular timer should be used?

This shouldn't even mention the driver, as the binding should describe
the HW, not how it's used by Linux at the moment.

> 
> Signed-off-by: Andrew Chew <achew@nvidia.com>
> ---
>  Documentation/devicetree/bindings/timer/nvidia,tegra30-timer.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/timer/nvidia,tegra30-timer.txt b/Documentation/devicetree/bindings/timer/nvidia,tegra30-timer.txt
> index b5082a1..e87fa70 100644
> --- a/Documentation/devicetree/bindings/timer/nvidia,tegra30-timer.txt
> +++ b/Documentation/devicetree/bindings/timer/nvidia,tegra30-timer.txt
> @@ -13,6 +13,13 @@ Required properties:
>  - clocks : Must contain one entry, for the module clock.
>    See ../clocks/clock-bindings.txt for details.
>  
> +Optional properties:
> +
> +- nvidia,wdt-timer-id: A list of timer IDs to be used for watchdogs.
> +    Watchdog 0 will be assigned to the first timer listed, watchdog 1 will
> +    be assigned to the second timer listed, etc. up to the number of watchdogs
> +    available.

This sounds like a description of what software should do. Is there any
reason this order is important?

Also, it feels odd for the proerty name to be singular given it's a
list...

Thanks,
Mark.

^ permalink raw reply

* Re: [PATCH v2 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Kevin Hilman @ 2014-02-03 17:39 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Brown, Mike Turquette, Emilio Lopez,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel,
	devicetree@vger.kernel.org, LKML,
	kevin.z.m.zh-Re5JQEeQqe8AvxtiuMwx3w,
	sunny-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf,
	shuge-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf,
	zhuzhenhua-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf
In-Reply-To: <20140131081147.GC2950@lukather>

Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> writes:

> Hi Kevin,
>
> On Thu, Jan 30, 2014 at 03:52:16PM -0800, Kevin Hilman wrote:
>> On Wed, Jan 29, 2014 at 5:32 AM, Maxime Ripard
>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> > On Wed, Jan 29, 2014 at 12:25:20PM +0000, Mark Brown wrote:
>> >> On Wed, Jan 29, 2014 at 12:10:48PM +0100, Maxime Ripard wrote:
>> >>
>> >> > +config SPI_SUN6I
>> >> > +   tristate "Allwinner A31 SPI controller"
>> >> > +   depends on ARCH_SUNXI || COMPILE_TEST
>> >> > +   select PM_RUNTIME
>> >> > +   help
>> >> > +     This enables using the SPI controller on the Allwinner A31 SoCs.
>> >> > +
>> >>
>> >> A select of PM_RUNTIME is both surprising and odd - why is that there?
>> >> The usual idiom is that the device starts out powered up (flagged using
>> >> pm_runtime_set_active()) and then runtime PM then suspends it when it's
>> >> compiled in.  That way if for some reason people want to avoid runtime
>> >> PM they can still use the device.
>> >
>> > Since pm_runtime_set_active and all the pm_runtime* callbacks in
>> > general are defined to pretty much empty functions, how the
>> > suspend/resume callbacks are called then? Obviously, we need them to
>> > be run, hence why I added the select here, but now I'm seeing a
>> > construct like what's following acceptable then?
>> 
>> Even with your 'select', The runtime PM callbacks will never be called
>> in the current driver.  pm_runtime_enable() doesn't do any runtime PM
>> transitions.  It just allows transitions to happen when they're
>> triggered by _get()/_put()/etc.
>
> Actually, pm_runtime_get_sync is called by the SPI framework whenever
> the device needs to be used. And pm_runtime_put whenever it's not used
> anymore, so the callbacks are actually called.

Ah, right.  I forgot that the SPI framework is using runtime PM now.

>> 
>> > pm_runtime_enable(&pdev->dev);
>> > if (!pm_runtime_enabled(&pdev->dev))
>> >    sun6i_spi_runtime_resume(&pdev->dev);
>> 
>> Similarily here, it's not the pm_runtime_enable that will fail when
>> runtime PM is disabled (or not built-in), it's a pm_runtime_get_sync()
>> that will fail.
>
> In the case where pm_runtime is disabled, pm_runtime_enabled will only
> return false, and hence the resume callback will be called. get_sync
> will fail too when the framework will call it, but since the device is
> already initialized, it's fine I guess.
>
>> What you want is something like this in ->probe()
>> 
>>    sun6i_spi_runtime_resume();
>>    /* now, device is always activated whether or not runtime PM is enabled */
>>    pm_runtime_enable();
>>    pm_runtime_set_active();  /* tells runtime PM core device is
>> already active */
>>    pm_runtime_get_sync();
>> 
>> This 'get' will increase the usecount, but not actually call the
>> callbacks because we told the RPM core that the device was already
>> activated with _set_active().
>> 
>> And then, in ->remove(), you'll want
>> 
>>    pm_runtime_put();
>>    pm_runtime_disable();
>> 
>> And if runtime PM is not enabled in the kernel, then the device will
>> be left on (which is kinda what you want if you didn't build runtime
>> PM into the kernel.)
>
> Yes, but that also mean that the device is actually on after the
> probe, even if it's never going to be used. From what I got reading
> the pm_runtime code, the suspend callback is called only whenever you
> call _put, so the device will be suspended only after it's been used
> the first time, right?
>
> Wouldn't it be better if it was suspended by default, and just waken
> up whenever the framework needs it?

Yes, but that's the job of runtime PM.  Without runtime PM, you have to
live with leaving the device powered up all the time.

Kevin

^ permalink raw reply

* Re: [PATCH] arm: document "mach-virt" platform.
From: Christoffer Dall @ 2014-02-03 17:41 UTC (permalink / raw)
  To: Christopher Covington
  Cc: Ian Campbell, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Arnd Bergmann, Pawel Moll, Stefano Stabellini, Marc Zyngier,
	Will Deacon, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Kumar Gala, Olof Johansson,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <52EF9D9F.5020600-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

On Mon, Feb 03, 2014 at 08:46:07AM -0500, Christopher Covington wrote:
> Hi Christoffer,
> 
> On 02/02/2014 11:56 PM, Christoffer Dall wrote:
> > On Thu, Jan 30, 2014 at 11:54:46AM -0500, Christopher Covington wrote:
> >> I think it would be informative to provide pointers here to commonly used
> >> paravirtualized devices, especially VirtIO PCI/MMIO.
> > 
> > I disagree: that would only encourage limited testing or assumptions
> > about these specific devices when really this platform is just a
> > bare-bones platform driven by device tree which should make no
> > preference, whatsoever, about which devices are used with the platform.
> 
> I'd be all for clearly stating that no assumptions can be made. Perhaps you
> can explain though how providing less documentation will result in more
> testing? The assertion does not currently make sense to me.
> 
If the documentation states that this is commonly used with virtio/xen
pv drivers, then I'm afraid people will just assume that's the only
devices the platform is used with.

Any mention of specific devices or features steers the attention away
from the essential point of this documentation.

I don't see how adding this text helps.  Such documentation should go
elsewhere, in QEMU, or a Xen/KVM web page or something like that.

Anyway, I'm not religious about this point.

-Christoffer
--
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] [RFC] Support for creating generic host_bridge from device tree
From: Liviu Dudau @ 2014-02-03 18:33 UTC (permalink / raw)
  To: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon
  Cc: LKML, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, LAKML,
	linaro-kernel

Following the discussion started here [1], I now have a proposal for tackling
generic support for host bridges described via device tree. It is an initial
stab at it, to try to get feedback and suggestions, but it is functional enough
that I have PCI Express for arm64 working on an FPGA using the patch that I am
also publishing that adds support for PCI for that platform.

Looking at the existing architectures that fit the requirements (use of device
tree and PCI) yields the powerpc and microblaze as generic enough to make them
candidates for conversion. I have a tentative patch for microblaze that I can
only compile test it, unfortunately using qemu-microblaze leads to an early
crash in the kernel.

As Bjorn has mentioned in the previous discussion, the idea is to add to
struct pci_host_bridge enough data to be able to reduce the size or remove the
architecture specific pci_controller structure. arm64 support actually manages
to get rid of all the architecture static data and has no pci_controller structure
defined. For host bridge drivers that means a change of API unless architectures
decide to provide a compatibility layer (comments here please).

In order to initialise a host bridge with the new API, the following example
code is sufficient for a _probe() function:

static int myhostbridge_probe(struct platform_device *pdev)
{
	int err;
	struct device_node *dev;
	struct pci_host_bridge *bridge;
	struct resource bus_range;
	struct myhostbridge_port *pp;
	LIST_HEAD(resources);

	dev = pdev->dev.of_node;

	if (!of_device_is_available(dev)) {
		pr_warn("%s: disabled\n", dev->full_name);
		return -ENODEV;
	}

	pp = kzalloc(sizeof(struct myhostbridge_port), GFP_KERNEL);
	if (!pp)
		return -ENOMEM;

	err = of_pci_parse_bus_range(dev, &bus_range);
	if (err) {
		bus_range.start = 0;
		bus_range.end = 255;
		bus_range.flags = IORESOURCE_BUS;
	}
	pci_add_resource(&resources, &bus_range);

	bridge = pci_host_bridge_of_init(&pdev->dev, 0, &myhostbridge_ops, pp, &resources);
	if (!bridge) {
		err = -EINVAL;
		goto bridge_init_fail;
	}

	err = myhostbridge_setup(bridge->bus);
	if (err)
		goto bridge_init_fail;

	/*
	 * Add flags here, this is just an example
	 */
	pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0);
	pci_add_flags(PCI_REASSIGN_ALL_BUS | PCI_REASSIGN_ALL_RSRC);

	bus_range.end = pci_scan_child_bus(bridge->bus);
	pci_bus_update_busn_res_end(bridge->bus, bus_range.end);

	pci_assign_unassigned_bus_resources(bridge->bus);

	pci_bus_add_devices(bridge->bus);

	return 0;

bridge_init_fail:
	kfree(pp);
	pci_free_resource_list(&resources);
	return err;
}

Best regards,
Liviu Dudau

[1] http://thread.gmane.org/gmane.linux.kernel.pci/25946

Liviu Dudau (1):
  pci: Add support for creating a generic host_bridge from device tree

 drivers/pci/host-bridge.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c       | 11 ++++++
 include/linux/pci.h       | 14 ++++++++
 3 files changed, 117 insertions(+)

-- 
1.8.5.3

--
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] pci: Add support for creating a generic host_bridge from device tree
From: Liviu Dudau @ 2014-02-03 18:33 UTC (permalink / raw)
  To: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon
  Cc: LKML, devicetree@vger.kernel.org, LAKML, linaro-kernel
In-Reply-To: <1391452428-22917-1-git-send-email-Liviu.Dudau@arm.com>

Several platforms use a rather generic version of parsing
the device tree to find the host bridge ranges. Move that
into the generic PCI code and use it to create a pci_host_bridge
structure that can be used by arch code.

Based on early attempts by Andrew Murray to unify the code.
Used powerpc and microblaze PCI code as starting point.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Catalin Marinas <Catalin.Marinas@arm.com>
Cc: Will Deacon <Will.Deacon@arm.com>
---
 drivers/pci/host-bridge.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/probe.c       | 11 ++++++
 include/linux/pci.h       | 14 ++++++++
 3 files changed, 117 insertions(+)

diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index 06ace62..9d11deb 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -6,6 +6,7 @@
 #include <linux/init.h>
 #include <linux/pci.h>
 #include <linux/module.h>
+#include <linux/of_address.h>
 
 #include "pci.h"
 
@@ -91,3 +92,94 @@ void pcibios_bus_to_resource(struct pci_bus *bus, struct resource *res,
 	res->end = region->end + offset;
 }
 EXPORT_SYMBOL(pcibios_bus_to_resource);
+
+/**
+ * pci_host_bridge_of_get_ranges - Parse PCI host bridge resources from DT
+ * @dev: device node of the host bridge having the range property
+ * @resources: list where the range of resources will be added after DT parsing
+ *
+ * This function will parse the "ranges" property of a PCI host bridge device
+ * node and setup the resource mapping based on its content. It is expected
+ * that the property conforms with the Power ePAPR document.
+ *
+ * Each architecture will then apply their filtering based on the limitations
+ * of each platform. One general restriction seems to be the number of IO space
+ * ranges, the PCI framework makes intensive use of struct resource management,
+ * and for IORESOURCE_IO types they can only be requested if they are contained
+ * within the global ioport_resource, so that should be limited to one IO space
+ * range.
+ */
+static int pci_host_bridge_of_get_ranges(struct device_node *dev,
+					struct list_head *resources)
+{
+	struct resource *res;
+	struct of_pci_range range;
+	struct of_pci_range_parser parser;
+	int err;
+
+	pr_info("PCI host bridge %s ranges:\n", dev->full_name);
+
+	/* Check for ranges property */
+	err = of_pci_range_parser_init(&parser, dev);
+	if (err)
+		return err;
+
+	pr_debug("Parsing ranges property...\n");
+	for_each_of_pci_range(&parser, &range) {
+		/* Read next ranges element */
+		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
+				range.pci_space, range.pci_addr);
+		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
+					range.cpu_addr, range.size);
+
+		/* If we failed translation or got a zero-sized region
+		 * (some FW try to feed us with non sensical zero sized regions
+		 * such as power3 which look like some kind of attempt
+		 * at exposing the VGA memory hole) then skip this range
+		 */
+		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
+			continue;
+
+		res = kzalloc(sizeof(struct resource), GFP_KERNEL);
+		if (!res) {
+			err = -ENOMEM;
+			goto bridge_ranges_nomem;
+		}
+
+		of_pci_range_to_resource(&range, dev, res);
+
+		pci_add_resource_offset(resources, res,
+				range.cpu_addr - range.pci_addr);
+	}
+
+	/* Apply architecture specific fixups for the ranges */
+	pcibios_fixup_bridge_ranges(resources);
+
+	return 0;
+
+bridge_ranges_nomem:
+	pci_free_resource_list(resources);
+	return err;
+}
+
+struct pci_host_bridge *
+pci_host_bridge_of_init(struct device *parent, int busno, struct pci_ops *ops,
+			void *host_data, struct list_head *resources)
+{
+	struct pci_bus *root_bus;
+	struct pci_host_bridge *bridge;
+
+	/* first parse the host bridge bus ranges */
+	if (pci_host_bridge_of_get_ranges(parent->of_node, resources))
+		return NULL;
+
+	/* then create the root bus */
+	root_bus = pci_create_root_bus(parent, busno, ops, host_data, resources);
+	if (!root_bus)
+		return NULL;
+
+	bridge = to_pci_host_bridge(root_bus->bridge);
+
+	return bridge;
+}
+EXPORT_SYMBOL(pci_host_bridge_of_init);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6e34498..16febae 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1787,6 +1787,17 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	list_for_each_entry_safe(window, n, resources, list) {
 		list_move_tail(&window->list, &bridge->windows);
 		res = window->res;
+		/*
+		 * IO resources are stored in the kernel with a CPU start
+		 * address of zero. Adjust the data accordingly and remember
+		 * the offset
+		 */
+		if (resource_type(res) == IORESOURCE_IO) {
+			bridge->io_offset = res->start;
+			res->end -= res->start;
+			window->offset -= res->start;
+			res->start = 0;
+		}
 		offset = window->offset;
 		if (res->flags & IORESOURCE_BUS)
 			pci_bus_insert_busn_res(b, bus, res->end);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index fb57c89..8953997 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -394,6 +394,8 @@ struct pci_host_bridge_window {
 struct pci_host_bridge {
 	struct device dev;
 	struct pci_bus *bus;		/* root bus */
+	resource_size_t io_offset;	/* CPU address offset for io resources */
+	int domain_nr;
 	struct list_head windows;	/* pci_host_bridge_windows */
 	void (*release_fn)(struct pci_host_bridge *);
 	void *release_data;
@@ -1762,11 +1764,23 @@ static inline struct device_node *pci_bus_to_OF_node(struct pci_bus *bus)
 	return bus ? bus->dev.of_node : NULL;
 }
 
+struct pci_host_bridge *
+pci_host_bridge_of_init(struct device *parent, int busno, struct pci_ops *ops,
+			void *host_data, struct list_head *resources);
+
+void pcibios_fixup_bridge_ranges(struct list_head *resources);
 #else /* CONFIG_OF */
 static inline void pci_set_of_node(struct pci_dev *dev) { }
 static inline void pci_release_of_node(struct pci_dev *dev) { }
 static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
 static inline void pci_release_bus_of_node(struct pci_bus *bus) { }
+
+static inline struct pci_host_bridge *
+pci_host_bridge_of_init(struct device *parent, struct pci_ops *ops,
+			void *host_data, struct list_head *resources)
+{
+	return NULL;
+}
 #endif  /* CONFIG_OF */
 
 #ifdef CONFIG_EEH
-- 
1.8.5.3

^ permalink raw reply related

* [PATCH] [RFC] Add AArch64 support for PCI
From: Liviu Dudau @ 2014-02-03 18:43 UTC (permalink / raw)
  To: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon
  Cc: devicetree@vger.kernel.org, linaro-kernel, Arnd Bergmann, LKML,
	LAKML

Hi,

This patch adds support for PCI to AArch64. It is based on the patch posted
here [1] that adds support for creating generic host bridge structure from
device tree. With that in place, I was able to boot an FPGA platform that
has PCIe host bridge support and use a PCIe network card.

The API used is different from the one used by ARM architecture. There is
no pci_common_init_dev() function and no hw_pci structure, as that is no
longer needed. Once the last signature is added to the legal agreement, I
will post the host bridge driver code that I am using. Meanwhile, here
is an example of what the probe function looks like, posted as an example:

static int myhostbridge_probe(struct platform_device *pdev)
{
	int err;
	struct device_node *dev;
	struct pci_host_bridge *bridge;
	struct resource bus_range;
	struct myhostbridge_port *pp;
	LIST_HEAD(resources);

	dev = pdev->dev.of_node;

	if (!of_device_is_available(dev)) {
		pr_warn("%s: disabled\n", dev->full_name);
		return -ENODEV;
	}

	pp = kzalloc(sizeof(struct myhostbridge_port), GFP_KERNEL);
	if (!pp)
		return -ENOMEM;

	err = of_pci_parse_bus_range(dev, &bus_range);
	if (err) {
		bus_range.start = 0;
		bus_range.end = 255;
		bus_range.flags = IORESOURCE_BUS;
	}
	pci_add_resource(&resources, &bus_range);

	bridge = pci_host_bridge_of_init(&pdev->dev, 0, &myhostbridge_ops, pp, &resources);
	if (!bridge) {
		err = -EINVAL;
		goto bridge_init_fail;
	}

	err = myhostbridge_setup(bridge->bus);
	if (err)
		goto bridge_init_fail;

	/*
	 * Add flags here, this is just an example
	 */
	pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0);
	pci_add_flags(PCI_REASSIGN_ALL_BUS | PCI_REASSIGN_ALL_RSRC);

	bus_range.end = pci_scan_child_bus(bridge->bus);
	pci_bus_update_busn_res_end(bridge->bus, bus_range.end);

	pci_assign_unassigned_bus_resources(bridge->bus);

	pci_bus_add_devices(bridge->bus);

	return 0;

bridge_init_fail:
	kfree(pp);
	pci_free_resource_list(&resources);
	return err;
}

Best regards,
Liviu

[1] http://marc.info/?l=linux-pci&m=139145254821334&w=2

Liviu Dudau (1):
  arm64: Add support for PCI

 arch/arm64/Kconfig            |  17 +++++++
 arch/arm64/include/asm/Kbuild |   1 +
 arch/arm64/include/asm/io.h   |   4 ++
 arch/arm64/include/asm/pci.h  |  35 +++++++++++++
 arch/arm64/kernel/Makefile    |   1 +
 arch/arm64/kernel/pci.c       | 112 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 170 insertions(+)
 create mode 100644 arch/arm64/include/asm/pci.h
 create mode 100644 arch/arm64/kernel/pci.c

-- 
1.8.5.3

^ permalink raw reply

* [PATCH] arm64: Add architecture support for PCI
From: Liviu Dudau @ 2014-02-03 18:43 UTC (permalink / raw)
  To: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon
  Cc: LKML, devicetree@vger.kernel.org, LAKML, linaro-kernel,
	Arnd Bergmann
In-Reply-To: <1391453028-23191-1-git-send-email-Liviu.Dudau@arm.com>

Use the generic host bridge functions to provide support for
PCI Express on arm64. There is no support for ISA memory.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---
 arch/arm64/Kconfig            |  17 +++++++
 arch/arm64/include/asm/Kbuild |   1 +
 arch/arm64/include/asm/io.h   |   4 ++
 arch/arm64/include/asm/pci.h  |  35 +++++++++++++
 arch/arm64/kernel/Makefile    |   1 +
 arch/arm64/kernel/pci.c       | 112 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 170 insertions(+)
 create mode 100644 arch/arm64/include/asm/pci.h
 create mode 100644 arch/arm64/kernel/pci.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index f8e5ee6..48fdd69 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -133,6 +133,23 @@ menu "Bus support"
 config ARM_AMBA
 	bool
 
+config PCI
+	bool "PCI support"
+	help
+	  This feature enables support for PCIe bus system. If you say Y
+	  here, the kernel will include drivers and infrastructure code
+	  to support PCIe bus devices.
+
+config PCI_DOMAINS
+	def_bool PCI
+
+config PCI_SYSCALL
+	def_bool PCI
+
+source "drivers/pci/Kconfig"
+source "drivers/pci/pcie/Kconfig"
+source "drivers/pci/hotplug/Kconfig"
+
 endmenu
 
 menu "Kernel Features"
diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
index 71c53ec..46924bc 100644
--- a/arch/arm64/include/asm/Kbuild
+++ b/arch/arm64/include/asm/Kbuild
@@ -26,6 +26,7 @@ generic-y += mman.h
 generic-y += msgbuf.h
 generic-y += mutex.h
 generic-y += pci.h
+generic-y += pci-bridge.h
 generic-y += poll.h
 generic-y += posix_types.h
 generic-y += resource.h
diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index 4cc813e..ce5bad2 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -120,9 +120,13 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
 /*
  *  I/O port access primitives.
  */
+#define arch_has_dev_port()	(0)
 #define IO_SPACE_LIMIT		0xffff
 #define PCI_IOBASE		((void __iomem *)(MODULES_VADDR - SZ_2M))
 
+#define ioport_map(port, nr)	(PCI_IOBASE + ((port) & IO_SPACE_LIMIT))
+#define ioport_unmap(addr)
+
 static inline u8 inb(unsigned long addr)
 {
 	return readb(addr + PCI_IOBASE);
diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
new file mode 100644
index 0000000..dd084a3
--- /dev/null
+++ b/arch/arm64/include/asm/pci.h
@@ -0,0 +1,35 @@
+#ifndef __ASM_PCI_H
+#define __ASM_PCI_H
+#ifdef __KERNEL__
+
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/dma-mapping.h>
+
+#include <asm/io.h>
+#include <asm-generic/pci-bridge.h>
+#include <asm-generic/pci-dma-compat.h>
+
+#define PCIBIOS_MIN_IO		0
+#define PCIBIOS_MIN_MEM		0
+
+/*
+ * Set to 1 if the kernel should re-assign all PCI bus numbers
+ */
+#define pcibios_assign_all_busses() \
+	(pci_has_flag(PCI_REASSIGN_ALL_BUS))
+
+/*
+ * PCI address space differs from physical memory address space
+ */
+#define PCI_DMA_BUS_IS_PHYS	(0)
+
+extern int isa_dma_bridge_buggy;
+
+extern int pci_domain_nr(struct pci_bus *bus);
+extern int pci_proc_domain(struct pci_bus *bus);
+
+extern int pci_ioremap_io(unsigned int offset, phys_addr_t phys_addr);
+
+#endif  /* __KERNEL__ */
+#endif  /* __ASM_PCI_H */
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 2d7fcc1..8cfec47 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -21,6 +21,7 @@ arm64-obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
 arm64-obj-$(CONFIG_ARM64_CPU_SUSPEND)	+= sleep.o suspend.o
 arm64-obj-$(CONFIG_JUMP_LABEL)		+= jump_label.o
 arm64-obj-$(CONFIG_SCHED_HMP)		+= sched_hmp.o
+arm64-obj-$(CONFIG_PCI)			+= pci.o
 
 obj-y					+= $(arm64-obj-y) vdso/
 obj-m					+= $(arm64-obj-m)
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
new file mode 100644
index 0000000..7b652cf
--- /dev/null
+++ b/arch/arm64/kernel/pci.c
@@ -0,0 +1,112 @@
+/*
+ * Code borrowed from powerpc/kernel/pci-common.c
+ *
+ * Copyright (C) 2003 Anton Blanchard <anton@au.ibm.com>, IBM
+ * Copyright (C) 2014 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+
+#include <asm/pci-bridge.h>
+
+
+/*
+ * Return the domain number for this bus
+ */
+int pci_domain_nr(struct pci_bus *bus)
+{
+	struct pci_host_bridge *bridge = to_pci_host_bridge(bus->bridge);
+
+	if (bridge)
+		return bridge->domain_nr;
+
+	return 0;
+}
+
+int pci_proc_domain(struct pci_bus *bus)
+{
+	return pci_domain_nr(bus);
+}
+
+/*
+ * Called after each bus is probed, but before its children are examined
+ */
+void pcibios_fixup_bus(struct pci_bus *bus)
+{
+	struct pci_dev *dev;
+	struct resource *res;
+	int i;
+
+	if (bus->self != NULL) {
+		pci_read_bridge_bases(bus);
+
+		pci_bus_for_each_resource(bus, res, i) {
+			if (!res || !res->flags || res->parent)
+				continue;
+
+			/*
+			 * If we are going to reassign everything, we can
+			 * shrink the P2P resource to have zero size to
+			 * save space
+			 */
+			if (pci_has_flag(PCI_REASSIGN_ALL_RSRC)) {
+				res->flags |= IORESOURCE_UNSET;
+				res->start = 0;
+				res->end = -1;
+				continue;
+			}
+		}
+	}
+
+	list_for_each_entry(dev, &bus->devices, bus_list) {
+		/* Ignore fully discovered devices */
+		if (dev->is_added)
+			continue;
+
+		set_dev_node(&dev->dev, pcibus_to_node(dev->bus));
+
+		/* Read default IRQs and fixup if necessary */
+		dev->irq = of_irq_parse_and_map_pci(dev, 0, 0);
+	}
+}
+EXPORT_SYMBOL(pcibios_fixup_bus);
+
+/*
+ * We don't have to worry about legacy ISA devices, so nothing to do here
+ */
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
+				resource_size_t size, resource_size_t align)
+{
+	return ALIGN(res->start, align);
+}
+EXPORT_SYMBOL(pcibios_align_resource);
+
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+	return pci_enable_resources(dev, mask);
+}
+
+void pcibios_fixup_bridge_ranges(struct list_head *resources)
+{
+}
+
+int pci_ioremap_io(unsigned int offset, phys_addr_t phys_addr)
+{
+	BUG_ON(offset + SZ_64K - 1 > IO_SPACE_LIMIT);
+
+	return ioremap_page_range((unsigned long)PCI_IOBASE + offset,
+				(unsigned long)PCI_IOBASE + offset + SZ_64K,
+				phys_addr,
+				__pgprot(PROT_DEVICE_nGnRE));
+}
-- 
1.8.5.3

^ permalink raw reply related

* Re: [PATCH] pci: Add support for creating a generic host_bridge from device tree
From: Arnd Bergmann @ 2014-02-03 18:46 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, LKML,
	devicetree@vger.kernel.org, LAKML, linaro-kernel
In-Reply-To: <1391452428-22917-2-git-send-email-Liviu.Dudau@arm.com>

On Monday 03 February 2014 18:33:48 Liviu Dudau wrote:
> +/**
> + * pci_host_bridge_of_get_ranges - Parse PCI host bridge resources from DT
> + * @dev: device node of the host bridge having the range property
> + * @resources: list where the range of resources will be added after DT parsing
> + *
> + * This function will parse the "ranges" property of a PCI host bridge device
> + * node and setup the resource mapping based on its content. It is expected
> + * that the property conforms with the Power ePAPR document.
> + *
> + * Each architecture will then apply their filtering based on the limitations
> + * of each platform. One general restriction seems to be the number of IO space
> + * ranges, the PCI framework makes intensive use of struct resource management,
> + * and for IORESOURCE_IO types they can only be requested if they are contained
> + * within the global ioport_resource, so that should be limited to one IO space
> + * range.

Actually we have quite a different set of restrictions around I/O space on ARM32
at the moment: Each host bridge can have its own 64KB range in an arbitrary
location on MMIO space, and the total must not exceed 2MB of I/O space.

> + */
> +static int pci_host_bridge_of_get_ranges(struct device_node *dev,
> +					struct list_head *resources)
> +{
> +	struct resource *res;
> +	struct of_pci_range range;
> +	struct of_pci_range_parser parser;
> +	int err;
> +
> +	pr_info("PCI host bridge %s ranges:\n", dev->full_name);
> +
> +	/* Check for ranges property */
> +	err = of_pci_range_parser_init(&parser, dev);
> +	if (err)
> +		return err;
> +
> +	pr_debug("Parsing ranges property...\n");
> +	for_each_of_pci_range(&parser, &range) {
> +		/* Read next ranges element */
> +		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
> +				range.pci_space, range.pci_addr);
> +		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
> +					range.cpu_addr, range.size);
> +
> +		/* If we failed translation or got a zero-sized region
> +		 * (some FW try to feed us with non sensical zero sized regions
> +		 * such as power3 which look like some kind of attempt
> +		 * at exposing the VGA memory hole) then skip this range
> +		 */
> +		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
> +			continue;
> +
> +		res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> +		if (!res) {
> +			err = -ENOMEM;
> +			goto bridge_ranges_nomem;
> +		}
> +
> +		of_pci_range_to_resource(&range, dev, res);
> +
> +		pci_add_resource_offset(resources, res,
> +				range.cpu_addr - range.pci_addr);
> +	}

I believe of_pci_range_to_resource() will return the MMIO aperture for the
I/O space window here, which is not what you are supposed to pass into
pci_add_resource_offset.

> +EXPORT_SYMBOL(pci_host_bridge_of_init);

EXPORT_SYMBOL_GPL

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 6e34498..16febae 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1787,6 +1787,17 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  	list_for_each_entry_safe(window, n, resources, list) {
>  		list_move_tail(&window->list, &bridge->windows);
>  		res = window->res;
> +		/*
> +		 * IO resources are stored in the kernel with a CPU start
> +		 * address of zero. Adjust the data accordingly and remember
> +		 * the offset
> +		 */
> +		if (resource_type(res) == IORESOURCE_IO) {
> +			bridge->io_offset = res->start;
> +			res->end -= res->start;
> +			window->offset -= res->start;
> +			res->start = 0;
> +		}
>  		offset = window->offset;
>  		if (res->flags & IORESOURCE_BUS)

Won't this break all existing host bridges?

	Arnd

^ permalink raw reply

* Re: [PATCH] arm64: Add architecture support for PCI
From: Arnd Bergmann @ 2014-02-03 18:58 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, LKML,
	devicetree@vger.kernel.org, LAKML, linaro-kernel
In-Reply-To: <1391453028-23191-2-git-send-email-Liviu.Dudau@arm.com>

On Monday 03 February 2014 18:43:48 Liviu Dudau wrote:
> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
> index 4cc813e..ce5bad2 100644
> --- a/arch/arm64/include/asm/io.h
> +++ b/arch/arm64/include/asm/io.h
> @@ -120,9 +120,13 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
>  /*
>   *  I/O port access primitives.
>   */
> +#define arch_has_dev_port()	(0)

Why not?

>  #define IO_SPACE_LIMIT		0xffff

You probably want to increase this a bit, to allow multiple host bridges
to have their own I/O space.

>  #define PCI_IOBASE		((void __iomem *)(MODULES_VADDR - SZ_2M))

And modify this location: There is no particular reason to have the I/O space
mapped exactly 2MB below the loadable modules, as virtual address space is
essentially free.

> +#define ioport_map(port, nr)	(PCI_IOBASE + ((port) & IO_SPACE_LIMIT))
> +#define ioport_unmap(addr)

inline functions?

>  static inline u8 inb(unsigned long addr)
>  {
>  	return readb(addr + PCI_IOBASE);
> diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
> new file mode 100644
> index 0000000..dd084a3
> --- /dev/null
> +++ b/arch/arm64/include/asm/pci.h
> @@ -0,0 +1,35 @@
> +#ifndef __ASM_PCI_H
> +#define __ASM_PCI_H
> +#ifdef __KERNEL__
> +
> +#include <linux/types.h>
> +#include <linux/slab.h>
> +#include <linux/dma-mapping.h>
> +
> +#include <asm/io.h>
> +#include <asm-generic/pci-bridge.h>
> +#include <asm-generic/pci-dma-compat.h>
> +
> +#define PCIBIOS_MIN_IO		0
> +#define PCIBIOS_MIN_MEM		0

PCIBIOS_MIN_IO is normally set to 0x1000, to stay out of the ISA range.

> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> new file mode 100644
> index 0000000..7b652cf
> --- /dev/null
> +++ b/arch/arm64/kernel/pci.c
> @@ -0,0 +1,112 @@

None of this looks really arm64 specific, nor should it be. I think
we should try a little harder to move this as a default implementation
into common code, even if we start out by having all architectures
override it.

> +int pci_ioremap_io(unsigned int offset, phys_addr_t phys_addr)
> +{
> +	BUG_ON(offset + SZ_64K - 1 > IO_SPACE_LIMIT);
> +
> +	return ioremap_page_range((unsigned long)PCI_IOBASE + offset,
> +				(unsigned long)PCI_IOBASE + offset + SZ_64K,
> +				phys_addr,
> +				__pgprot(PROT_DEVICE_nGnRE));
> +}

Not sure if we want to treat this one as architecture specific though.
It certainly won't be portable to x86, but it could be shared with
a couple of others. We may also want to redesign the interface.
I've been thinking we could make this function allocate space in the
Linux virtual I/O space aperture, and pass two resources into it
(physical I/O aperture and bus I/O range), and get the actual
io_offset as the return value, or a negative error number.

That way, you could have an arbitrary number of host bridges in the
system and each one gets a share of the virtual aperture until
it's full.

	Arnd

^ permalink raw reply

* RE: [PATCH v1] ARM: tegra: add nvidia,wdt-timer-id optional property
From: Andrew Chew @ 2014-02-03 18:59 UTC (permalink / raw)
  To: Mark Rutland
  Cc: robh+dt@kernel.org, Pawel Moll, ijc+devicetree@hellion.org.uk,
	galak@codeaurora.org, rob@landley.net, swarren@wwwdotorg.org,
	thierry.reding@gmail.com, abrestic@chromium.org,
	dgreid@chromium.org, katierh@chromium.org,
	devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20140203171053.GA28338@e106331-lin.cambridge.arm.com>

> On Fri, Jan 31, 2014 at 09:46:51PM +0000, Andrew Chew wrote:
> > This optional property can be used to specify which timers are to be
> > used for hardware watchdog timeouts (via a tegra wdt driver).
> 
> Is there any reason that a particular timer should be used?

I worry about colliding with other timer allocations, and wanted to be
flexible in this regard.

> This shouldn't even mention the driver, as the binding should describe the
> HW, not how it's used by Linux at the moment.
> 
> >
> > Signed-off-by: Andrew Chew <achew@nvidia.com>
> > ---
> >  Documentation/devicetree/bindings/timer/nvidia,tegra30-timer.txt | 8
> > ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/timer/nvidia,tegra30-timer.txt
> > b/Documentation/devicetree/bindings/timer/nvidia,tegra30-timer.txt
> > index b5082a1..e87fa70 100644
> > --- a/Documentation/devicetree/bindings/timer/nvidia,tegra30-timer.txt
> > +++ b/Documentation/devicetree/bindings/timer/nvidia,tegra30-timer.txt
> > @@ -13,6 +13,13 @@ Required properties:
> >  - clocks : Must contain one entry, for the module clock.
> >    See ../clocks/clock-bindings.txt for details.
> >
> > +Optional properties:
> > +
> > +- nvidia,wdt-timer-id: A list of timer IDs to be used for watchdogs.
> > +    Watchdog 0 will be assigned to the first timer listed, watchdog 1 will
> > +    be assigned to the second timer listed, etc. up to the number of
> watchdogs
> > +    available.
> 
> This sounds like a description of what software should do. Is there any
> reason this order is important?

The order in regards to which watchdog (watchdog 0, watchdog 1, etc) is paired
with which timer is unimportant for purposes of the watchdog driver that
I will follow up with.  I can leave those details out of the bindings description
if that resolves your concern.

> Also, it feels odd for the proerty name to be singular given it's a list...

You're right.  Given what it is, it really should be nvidia,wdt-timer-ids.


^ permalink raw reply

* Re: [PATCH] pci: Add support for creating a generic host_bridge from device tree
From: Liviu Dudau @ 2014-02-03 19:06 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, LKML,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, LAKML,
	linaro-kernel
In-Reply-To: <7398333.9L5KlyFggU@wuerfel>

Hi Arnd,

First of all, thanks for reviewing this!

On Mon, Feb 03, 2014 at 06:46:10PM +0000, Arnd Bergmann wrote:
> On Monday 03 February 2014 18:33:48 Liviu Dudau wrote:
> > +/**
> > + * pci_host_bridge_of_get_ranges - Parse PCI host bridge resources from DT
> > + * @dev: device node of the host bridge having the range property
> > + * @resources: list where the range of resources will be added after DT parsing
> > + *
> > + * This function will parse the "ranges" property of a PCI host bridge device
> > + * node and setup the resource mapping based on its content. It is expected
> > + * that the property conforms with the Power ePAPR document.
> > + *
> > + * Each architecture will then apply their filtering based on the limitations
> > + * of each platform. One general restriction seems to be the number of IO space
> > + * ranges, the PCI framework makes intensive use of struct resource management,
> > + * and for IORESOURCE_IO types they can only be requested if they are contained
> > + * within the global ioport_resource, so that should be limited to one IO space
> > + * range.
> 
> Actually we have quite a different set of restrictions around I/O space on ARM32
> at the moment: Each host bridge can have its own 64KB range in an arbitrary
> location on MMIO space, and the total must not exceed 2MB of I/O space.

And that is why the filtering is not (yet) imposed in the generic code. But once
you use pci_request_region, that will call request_region which will check
against ioport_resource as parent for the requested resource. That should fail
if is is not in the correct range, so I don't know how arm arch code manages
multiple IO ranges.

> 
> > + */
> > +static int pci_host_bridge_of_get_ranges(struct device_node *dev,
> > +					struct list_head *resources)
> > +{
> > +	struct resource *res;
> > +	struct of_pci_range range;
> > +	struct of_pci_range_parser parser;
> > +	int err;
> > +
> > +	pr_info("PCI host bridge %s ranges:\n", dev->full_name);
> > +
> > +	/* Check for ranges property */
> > +	err = of_pci_range_parser_init(&parser, dev);
> > +	if (err)
> > +		return err;
> > +
> > +	pr_debug("Parsing ranges property...\n");
> > +	for_each_of_pci_range(&parser, &range) {
> > +		/* Read next ranges element */
> > +		pr_debug("pci_space: 0x%08x pci_addr:0x%016llx ",
> > +				range.pci_space, range.pci_addr);
> > +		pr_debug("cpu_addr:0x%016llx size:0x%016llx\n",
> > +					range.cpu_addr, range.size);
> > +
> > +		/* If we failed translation or got a zero-sized region
> > +		 * (some FW try to feed us with non sensical zero sized regions
> > +		 * such as power3 which look like some kind of attempt
> > +		 * at exposing the VGA memory hole) then skip this range
> > +		 */
> > +		if (range.cpu_addr == OF_BAD_ADDR || range.size == 0)
> > +			continue;
> > +
> > +		res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> > +		if (!res) {
> > +			err = -ENOMEM;
> > +			goto bridge_ranges_nomem;
> > +		}
> > +
> > +		of_pci_range_to_resource(&range, dev, res);
> > +
> > +		pci_add_resource_offset(resources, res,
> > +				range.cpu_addr - range.pci_addr);
> > +	}
> 
> I believe of_pci_range_to_resource() will return the MMIO aperture for the
> I/O space window here, which is not what you are supposed to pass into
> pci_add_resource_offset.

And that is why the code in probe.c has been added to deal with that. It is
too early to do the adjustments here as all we have is the list of resources
and that might get culled by the architecture fixup code. Remembering the
io_offset will happen once the pci_host_bridge gets created, and the resources
are then adjusted.

> 
> > +EXPORT_SYMBOL(pci_host_bridge_of_init);
> 
> EXPORT_SYMBOL_GPL

Will change for v2, thanks!

> 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 6e34498..16febae 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1787,6 +1787,17 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> >  	list_for_each_entry_safe(window, n, resources, list) {
> >  		list_move_tail(&window->list, &bridge->windows);
> >  		res = window->res;
> > +		/*
> > +		 * IO resources are stored in the kernel with a CPU start
> > +		 * address of zero. Adjust the data accordingly and remember
> > +		 * the offset
> > +		 */
> > +		if (resource_type(res) == IORESOURCE_IO) {
> > +			bridge->io_offset = res->start;
> > +			res->end -= res->start;
> > +			window->offset -= res->start;
> > +			res->start = 0;
> > +		}
> >  		offset = window->offset;
> >  		if (res->flags & IORESOURCE_BUS)
> 
> Won't this break all existing host bridges?

I am not sure. I believe not, due to what I've explained earlier, but you might be right.

The adjustment happens before the resource is added to the host bridge windows and translates
it from MMIO range into IO range.

Best regards,
Liviu

> 
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

--
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] arm64: Add architecture support for PCI
From: Liviu Dudau @ 2014-02-03 19:18 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-pci, Bjorn Helgaas, Catalin Marinas, Will Deacon, LKML,
	devicetree@vger.kernel.org, LAKML, linaro-kernel
In-Reply-To: <21596846.kVTqp7roW4@wuerfel>

On Mon, Feb 03, 2014 at 06:58:56PM +0000, Arnd Bergmann wrote:
> On Monday 03 February 2014 18:43:48 Liviu Dudau wrote:
> > diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
> > index 4cc813e..ce5bad2 100644
> > --- a/arch/arm64/include/asm/io.h
> > +++ b/arch/arm64/include/asm/io.h
> > @@ -120,9 +120,13 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
> >  /*
> >   *  I/O port access primitives.
> >   */
> > +#define arch_has_dev_port()	(0)
> 
> Why not?

Maybe I got it the wrong way around, but the comment in include/linux/io.h says:

/*
 * Some systems do not have legacy ISA devices.
 * /dev/port is not a valid interface on these systems.
 * So for those archs, <asm/io.h> should define the following symbol.
 */

So ... defining it should mean no legacy ISA devices, right?

> 
> >  #define IO_SPACE_LIMIT		0xffff
> 
> You probably want to increase this a bit, to allow multiple host bridges
> to have their own I/O space.

OK, but to what size?

> 
> >  #define PCI_IOBASE		((void __iomem *)(MODULES_VADDR - SZ_2M))
> 
> And modify this location: There is no particular reason to have the I/O space
> mapped exactly 2MB below the loadable modules, as virtual address space is
> essentially free.

Will talk with Catalin about where to place this.

> 
> > +#define ioport_map(port, nr)	(PCI_IOBASE + ((port) & IO_SPACE_LIMIT))
> > +#define ioport_unmap(addr)
> 
> inline functions?

Will do, thanks!

> 
> >  static inline u8 inb(unsigned long addr)
> >  {
> >  	return readb(addr + PCI_IOBASE);
> > diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
> > new file mode 100644
> > index 0000000..dd084a3
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/pci.h
> > @@ -0,0 +1,35 @@
> > +#ifndef __ASM_PCI_H
> > +#define __ASM_PCI_H
> > +#ifdef __KERNEL__
> > +
> > +#include <linux/types.h>
> > +#include <linux/slab.h>
> > +#include <linux/dma-mapping.h>
> > +
> > +#include <asm/io.h>
> > +#include <asm-generic/pci-bridge.h>
> > +#include <asm-generic/pci-dma-compat.h>
> > +
> > +#define PCIBIOS_MIN_IO		0
> > +#define PCIBIOS_MIN_MEM		0
> 
> PCIBIOS_MIN_IO is normally set to 0x1000, to stay out of the ISA range.

:) No ISA support! (Die ISA, die!!) 

> 
> > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > new file mode 100644
> > index 0000000..7b652cf
> > --- /dev/null
> > +++ b/arch/arm64/kernel/pci.c
> > @@ -0,0 +1,112 @@
> 
> None of this looks really arm64 specific, nor should it be. I think
> we should try a little harder to move this as a default implementation
> into common code, even if we start out by having all architectures
> override it.

Agree. This is the RFC version. I didn't dare to post a patch with fixes
for all architectures. :)

> 
> > +int pci_ioremap_io(unsigned int offset, phys_addr_t phys_addr)
> > +{
> > +	BUG_ON(offset + SZ_64K - 1 > IO_SPACE_LIMIT);
> > +
> > +	return ioremap_page_range((unsigned long)PCI_IOBASE + offset,
> > +				(unsigned long)PCI_IOBASE + offset + SZ_64K,
> > +				phys_addr,
> > +				__pgprot(PROT_DEVICE_nGnRE));
> > +}
> 
> Not sure if we want to treat this one as architecture specific though.
> It certainly won't be portable to x86, but it could be shared with
> a couple of others. We may also want to redesign the interface.
> I've been thinking we could make this function allocate space in the
> Linux virtual I/O space aperture, and pass two resources into it
> (physical I/O aperture and bus I/O range), and get the actual
> io_offset as the return value, or a negative error number.

Not sure I completely follow your idea.

> 
> That way, you could have an arbitrary number of host bridges in the
> system and each one gets a share of the virtual aperture until
> it's full.

One still needs to fix the pci_request_region use that checks against
ioport_resource. But it is an interesting idea.

> 
> 	Arnd
> 
> 

Thanks for reviewing this patch!

Liviu

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox