* [PATCH 4/4] ARM: dts: sun8i: add OTG function to Lichee Pi Zero
From: Bin Liu @ 2017-01-13 21:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170112173938.5am5njwrbs6p62br@lukather>
On Thu, Jan 12, 2017 at 06:39:38PM +0100, Maxime Ripard wrote:
> Hi Bin,
>
> On Thu, Jan 12, 2017 at 08:50:14AM -0600, Bin Liu wrote:
> > On Wed, Jan 11, 2017 at 10:06:38PM +0100, Maxime Ripard wrote:
> > > On Wed, Jan 11, 2017 at 02:08:11PM -0600, Bin Liu wrote:
> > > > On Thu, Jan 12, 2017 at 03:55:33AM +0800, Icenowy Zheng wrote:
> > > > >
> > > > >
> > > > > 11.01.2017, 04:24, "Bin Liu" <b-liu@ti.com>:
> > > > > > On Tue, Jan 03, 2017 at 11:25:34PM +0800, Icenowy Zheng wrote:
> > > > > >> ?Lichee Pi Zero features a USB OTG port.
> > > > > >>
> > > > > >> ?Add support for it.
> > > > > >>
> > > > > >> ?Note: in order to use the Host mode, the board must be powered via the
> > > > > >> ?+5V and GND pins.
> > > > > >>
> > > > > >> ?Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> > > > > >> ?---
> > > > > >> ??arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts | 10 ++++++++++
> > > > > >> ??1 file changed, 10 insertions(+)
> > > > > >>
> > > > > >> ?diff --git a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >> ?index 0099affc6ce3..3d9168cbaeca 100644
> > > > > >> ?--- a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >> ?+++ b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >> ?@@ -71,3 +71,13 @@
> > > > > >> ??????????pinctrl-names = "default";
> > > > > >> ??????????status = "okay";
> > > > > >> ??};
> > > > > >> ?+
> > > > > >> ?+&usb_otg {
> > > > > >> ?+ dr_mode = "otg";
> > > > > >
> > > > > > Why not set this default mode in dtsi instead?
> > > > > >
> > > > > > Regards,
> > > > > > -Bin.
> > > > >
> > > > > There's possibly boards which do not have OTG functions.
> > > >
> > > > That is board specific.
> > >
> > > Exactly, and this is why it should be done in the board DT.
> >
> > I am just suggesting based on the common practice. If a .dtsi exists for
> > a family, the .dtsi describes the device and common properties for all
> > possible boards, and each board .dts adds or overrides its specific
> > implementation. Kernel has many devices/boards done in this way - define
> > the default dr_mode in .dtsi.
> >
> > In this case, I suggest to set the common dr_mode in .dtsi, then each
> > board .dts only overrides it if the implementation is different.
> >
> > >
> > > The controller in the Allwinner SoCs do not handle directly the ID pin
> > > and VBUS, but rather rely on a GPIO to do so.
> > >
> > > So boards with OTG will need setup anyway, at least to tell which
> > > GPIOs are used. There's no point in enforcing a default if it doesn't
> > > work by default.
> >
> > Then define a default which supposes to work for most boards.
> >
> > Why I suggest this, is because defining a default dr_mode which works
> > for most cases in dtsi could prevent a little surprise in MUSB function.
> > If someone designs a new board but forgets to define dr_mode in the new
> > board DT, the MUSB driver will default to org mode, which might not be
> > intended.
>
> The point is that there is no sensible default. Some boards don't have
> an ID pin and no VBUS (peripheral), some don't have an ID pin but VBUS
> (host), and some have an ID pin but no controllable VBUS, some have an
> ID pin and a controllable VBUS, but we have no idea which GPIOs are
> used.
>
> There's no way we can have something that works on most cases.
Ok, understood.
Regards,
-Bin.
^ permalink raw reply
* [PATCH net-next v2 08/10] net: dsa: Add support for platform data
From: Florian Fainelli @ 2017-01-13 22:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113140459.GH10203@lunn.ch>
On 01/13/2017 06:04 AM, Andrew Lunn wrote:
>> index cd91070b5467..d326fc4afad7 100644
>> --- a/net/dsa/dsa2.c
>> +++ b/net/dsa/dsa2.c
>> @@ -81,17 +81,23 @@ static void dsa_dst_del_ds(struct dsa_switch_tree *dst,
>>
>> static bool dsa_port_is_valid(struct dsa_port *port)
>> {
>> - return !!port->dn;
>> + return !!(port->dn || port->name);
>> }
>
> Does this clash with Viviens recent change to make names optional and
> have the kernel assign it?
So there were two ways to look at this, one was that could check here
that ds->pd is assigned and port->name is assigned, which means that
platform data has to provide valid port name. We can also eliminate this
check entirely because we now support NULL names just fines.
>
> I suppose you could use an name of "eth%d"? Is it worth adding a
> comment to the platform data structure?
Humm, that could be done, maybe for simplicity we can just let
net/dsa/dsa2.c assign names either based on what platform data provided,
or by falling back to eth%d.
Thanks!
--
Florian
^ permalink raw reply
* [PATCH net-next v2 08/10] net: dsa: Add support for platform data
From: Florian Fainelli @ 2017-01-13 22:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113141110.GI10203@lunn.ch>
On 01/13/2017 06:11 AM, Andrew Lunn wrote:
>> static int _dsa_register_switch(struct dsa_switch *ds, struct device *dev)
>> {
>> + struct dsa_chip_data *pdata = dev->platform_data;
>> struct device_node *np = dev->of_node;
>> struct dsa_switch_tree *dst;
>> struct device_node *ports;
>> u32 tree, index;
>> int i, err;
>>
>> - err = dsa_parse_member_dn(np, &tree, &index);
>> - if (err)
>> - return err;
>> + if (np) {
>> + err = dsa_parse_member_dn(np, &tree, &index);
>> + if (err)
>> + return err;
>>
>> - ports = dsa_get_ports(ds, np);
>> - if (IS_ERR(ports))
>> - return PTR_ERR(ports);
>> + ports = dsa_get_ports(ds, np);
>> + if (IS_ERR(ports))
>> + return PTR_ERR(ports);
>>
>> - err = dsa_parse_ports_dn(ports, ds);
>> - if (err)
>> - return err;
>> + err = dsa_parse_ports_dn(ports, ds);
>> + if (err)
>> + return err;
>> + } else {
>> + err = dsa_parse_member(pdata, &tree, &index);
>
Hello Andrew,
> Hi Florian
>
> Maybe it is hiding, but i don't see anywhere you check that pdata !=
> NULL.
You are right, there is not such a check, it should probably be added
early on.
>
> At least for x86 platforms, i don't expect we are booting using
> platform data like ARM systems used to do. I think it is more likely a
> glue module will be loaded. It looks up the MDIO bus and appends a
> platform data to an MDIO device. The switch driver then needs to load
> and use the platform data. But if things happen in a different order,
> it could be the switch driver probes before the glue driver, meaning
> pdata is NULL.
That's very valid, I will fix this, thanks!
>
> Do we even want to return -EPROBE_DEFERED?
I was trying to exercise that code path a little bit, but could not
quite make sense of what I was seeing, let me try again with more tracing.
--
Florian
^ permalink raw reply
* [PATCH v3 02/24] ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node
From: Steve Longerbeam @ 2017-01-13 22:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484308678.31475.24.camel@pengutronix.de>
On 01/13/2017 03:57 AM, Philipp Zabel wrote:
> Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
>> Add to the MIPI CSI2 receiver node: compatible string, interrupt sources,
>> clocks.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> ---
>> arch/arm/boot/dts/imx6qdl.dtsi | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
>> index 53e6e63..42926e9 100644
>> --- a/arch/arm/boot/dts/imx6qdl.dtsi
>> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
>> @@ -1125,7 +1125,14 @@
>> };
>>
>> mipi_csi: mipi at 021dc000 {
>> + compatible = "fsl,imx6-mipi-csi2";
>> reg = <0x021dc000 0x4000>;
>> + interrupts = <0 100 0x04>, <0 101 0x04>;
>> + clocks = <&clks IMX6QDL_CLK_HSI_TX>,
>> + <&clks IMX6QDL_CLK_VIDEO_27M>,
>> + <&clks IMX6QDL_CLK_EIM_SEL>;
> I think the latter should be EIM_PODF
done.
>
>> + clock-names = "dphy", "cfg", "pix";
> and I'm not sure dphy is the right name for this one. Is that the pll
> ref input?
I believe this naming came from FSL's mipi csi-2 driver. It is the "hsi_tx"
clock (presumably for the MIPI HSI controller) whose parents are selected
by the CDCDR register as PLL3_120M or PLL2_PFD2. I have no clue whether
this is indeed also used as the clock for the MIPI CSI-2 D-PHY, but
according
to FSL naming convention it might be.
Steve
^ permalink raw reply
* [PATCH v3 06/24] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors
From: Steve Longerbeam @ 2017-01-13 23:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484309021.31475.29.camel@pengutronix.de>
On 01/13/2017 04:03 AM, Philipp Zabel wrote:
> Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
>> Enables the OV5642 parallel-bus sensor, and the OV5640 MIPI CSI-2 sensor.
>> Both hang off the same i2c2 bus, so they require different (and non-
>> default) i2c slave addresses.
>>
>> The OV5642 connects to the parallel-bus mux input port on ipu1_csi0_mux.
>>
>> The OV5640 connects to the input port on the MIPI CSI-2 receiver on
>> mipi_csi. It is set to transmit over MIPI virtual channel 1.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> ---
>> arch/arm/boot/dts/imx6dl-sabrelite.dts | 5 ++
>> arch/arm/boot/dts/imx6q-sabrelite.dts | 6 ++
>> arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 118 +++++++++++++++++++++++++++++++
>> 3 files changed, 129 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx6dl-sabrelite.dts b/arch/arm/boot/dts/imx6dl-sabrelite.dts
>> index 0f06ca5..fec2524 100644
>> --- a/arch/arm/boot/dts/imx6dl-sabrelite.dts
>> +++ b/arch/arm/boot/dts/imx6dl-sabrelite.dts
>> @@ -48,3 +48,8 @@
>> model = "Freescale i.MX6 DualLite SABRE Lite Board";
>> compatible = "fsl,imx6dl-sabrelite", "fsl,imx6dl";
>> };
>> +
>> +&ipu1_csi1_from_ipu1_csi1_mux {
>> + data-lanes = <0 1>;
>> + clock-lanes = <2>;
>> +};
>> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
>> index 66d10d8..9e2d26d 100644
>> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
>> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
>> @@ -52,3 +52,9 @@
>> &sata {
>> status = "okay";
>> };
>> +
>> +&ipu1_csi1_from_mipi_vc1 {
>> + data-lanes = <0 1>;
>> + clock-lanes = <2>;
>> +};
>> +
>> diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> index 795b5a5..bca9fed 100644
>> --- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> +++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
>> @@ -39,6 +39,8 @@
>> * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> * OTHER DEALINGS IN THE SOFTWARE.
>> */
>> +
>> +#include <dt-bindings/clock/imx6qdl-clock.h>
>> #include <dt-bindings/gpio/gpio.h>
>> #include <dt-bindings/input/input.h>
>>
>> @@ -96,6 +98,15 @@
>> };
>> };
>>
>> + mipi_xclk: mipi_xclk {
>> + compatible = "pwm-clock";
>> + #clock-cells = <0>;
>> + clock-frequency = <22000000>;
>> + clock-output-names = "mipi_pwm3";
>> + pwms = <&pwm3 0 45>; /* 1 / 45 ns = 22 MHz */
>> + status = "okay";
>> + };
>> +
>> gpio-keys {
>> compatible = "gpio-keys";
>> pinctrl-names = "default";
>> @@ -220,6 +231,22 @@
>> };
>> };
>>
>> +&ipu1_csi0_from_ipu1_csi0_mux {
>> + bus-width = <8>;
>> + data-shift = <12>; /* Lines 19:12 used */
>> + hsync-active = <1>;
>> + vync-active = <1>;
>> +};
>> +
>> +&ipu1_csi0_mux_from_parallel_sensor {
>> + remote-endpoint = <&ov5642_to_ipu1_csi0_mux>;
>> +};
>> +
>> +&ipu1_csi0 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_ipu1_csi0>;
>> +};
>> +
>> &audmux {
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_audmux>;
>> @@ -299,6 +326,52 @@
>> pinctrl-names = "default";
>> pinctrl-0 = <&pinctrl_i2c2>;
>> status = "okay";
>> +
>> + ov5640: camera at 40 {
>> + compatible = "ovti,ov5640";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_ov5640>;
>> + clocks = <&mipi_xclk>;
>> + clock-names = "xclk";
>> + reg = <0x40>;
>> + xclk = <22000000>;
> This is superfluous, you can use clk_get_rate on mipi_xclk.
This property is actually there to tell the driver what to set the
rate to, with clk_set_rate(). So you are saying it would be better
to set the rate in the device tree and the driver should only
retrieve the rate?
Steve
^ permalink raw reply
* [PATCH v8 16/18] vfio/type1: Allow transparent MSI IOVA allocation
From: Alex Williamson @ 2017-01-13 23:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484127714-3263-17-git-send-email-eric.auger@redhat.com>
On Wed, 11 Jan 2017 09:41:52 +0000
Eric Auger <eric.auger@redhat.com> wrote:
> When attaching a group to the container, check the group's
> reserved regions and test whether the IOMMU translates MSI
> transactions. If yes, we initialize an IOVA allocator through
> the iommu_get_msi_cookie API. This will allow the MSI IOVAs
> to be transparently allocated on MSI controller's compose().
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>
> v3 -> v4:
> - test region's type: IOMMU_RESV_MSI
> - restructure the code to prepare for safety assessment
> - reword title
> ---
> drivers/vfio/vfio_iommu_type1.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 9266271..5651faf 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -39,6 +39,7 @@
> #include <linux/pid_namespace.h>
> #include <linux/mdev.h>
> #include <linux/notifier.h>
> +#include <linux/dma-iommu.h>
>
> #define DRIVER_VERSION "0.2"
> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
> @@ -1181,6 +1182,28 @@ static struct vfio_group *find_iommu_group(struct vfio_domain *domain,
> return NULL;
> }
>
> +static bool vfio_iommu_has_resv_msi(struct iommu_group *group,
> + phys_addr_t *base)
> +{
> + struct list_head group_resv_regions;
> + struct iommu_resv_region *region, *next;
> + bool ret = false;
> +
> + INIT_LIST_HEAD(&group_resv_regions);
> + iommu_get_group_resv_regions(group, &group_resv_regions);
> + list_for_each_entry(region, &group_resv_regions, list) {
> + if (region->type & IOMMU_RESV_MSI) {
> + *base = region->start;
> + ret = true;
> + goto out;
> + }
> + }
> +out:
> + list_for_each_entry_safe(region, next, &group_resv_regions, list)
> + kfree(region);
> + return ret;
> +}
> +
> static int vfio_iommu_type1_attach_group(void *iommu_data,
> struct iommu_group *iommu_group)
> {
> @@ -1189,6 +1212,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> struct vfio_domain *domain, *d;
> struct bus_type *bus = NULL, *mdev_bus;
> int ret;
> + bool resv_msi;
> + phys_addr_t resv_msi_base;
>
> mutex_lock(&iommu->lock);
>
> @@ -1258,6 +1283,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> if (ret)
> goto out_domain;
>
> + resv_msi = vfio_iommu_has_resv_msi(iommu_group, &resv_msi_base);
> +
> INIT_LIST_HEAD(&domain->group_list);
> list_add(&group->next, &domain->group_list);
>
> @@ -1304,6 +1331,9 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> if (ret)
> goto out_detach;
>
> + if (resv_msi && iommu_get_msi_cookie(domain->domain, resv_msi_base))
> + goto out_detach;
> +
> list_add(&domain->next, &iommu->domain_list);
>
> mutex_unlock(&iommu->lock);
^ permalink raw reply
* [PATCH v8 17/18] vfio/type1: Check MSI remapping at irq domain level
From: Alex Williamson @ 2017-01-13 23:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484127714-3263-18-git-send-email-eric.auger@redhat.com>
On Wed, 11 Jan 2017 09:41:53 +0000
Eric Auger <eric.auger@redhat.com> wrote:
> In case the IOMMU translates MSI transactions (typical case
> on ARM), we check MSI remapping capability at IRQ domain
> level. Otherwise it is checked at IOMMU level.
>
> At this stage the arm-smmu-(v3) still advertise the
> IOMMU_CAP_INTR_REMAP capability at IOMMU level. This will be
> removed in subsequent patches.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
Acked-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>
> v6: rewrite test
> ---
> drivers/vfio/vfio_iommu_type1.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 5651faf..ec903a0 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -40,6 +40,7 @@
> #include <linux/mdev.h>
> #include <linux/notifier.h>
> #include <linux/dma-iommu.h>
> +#include <linux/irqdomain.h>
>
> #define DRIVER_VERSION "0.2"
> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
> @@ -1212,7 +1213,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> struct vfio_domain *domain, *d;
> struct bus_type *bus = NULL, *mdev_bus;
> int ret;
> - bool resv_msi;
> + bool resv_msi, msi_remap;
> phys_addr_t resv_msi_base;
>
> mutex_lock(&iommu->lock);
> @@ -1288,8 +1289,10 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> INIT_LIST_HEAD(&domain->group_list);
> list_add(&group->next, &domain->group_list);
>
> - if (!allow_unsafe_interrupts &&
> - !iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) {
> + msi_remap = resv_msi ? irq_domain_check_msi_remap() :
> + iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
> +
> + if (!allow_unsafe_interrupts && !msi_remap) {
> pr_warn("%s: No interrupt remapping support. Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n",
> __func__);
> ret = -EPERM;
^ permalink raw reply
* [PATCH v3 15/24] media: Add userspace header file for i.MX
From: Steve Longerbeam @ 2017-01-13 23:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484309143.31475.31.camel@pengutronix.de>
On 01/13/2017 04:05 AM, Philipp Zabel wrote:
> Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
>> This adds a header file for use by userspace programs wanting to interact
>> with the i.MX media driver. It defines custom v4l2 controls and events
>> generated by the i.MX v4l2 subdevices.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> ---
>> include/uapi/media/Kbuild | 1 +
>> include/uapi/media/imx.h | 30 ++++++++++++++++++++++++++++++
>> 2 files changed, 31 insertions(+)
>> create mode 100644 include/uapi/media/imx.h
>>
>> diff --git a/include/uapi/media/Kbuild b/include/uapi/media/Kbuild
>> index aafaa5a..fa78958 100644
>> --- a/include/uapi/media/Kbuild
>> +++ b/include/uapi/media/Kbuild
>> @@ -1 +1,2 @@
>> # UAPI Header export list
>> +header-y += imx.h
>> diff --git a/include/uapi/media/imx.h b/include/uapi/media/imx.h
>> new file mode 100644
>> index 0000000..2421d9c
>> --- /dev/null
>> +++ b/include/uapi/media/imx.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Copyright (c) 2014-2015 Mentor Graphics Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by the
>> + * Free Software Foundation; either version 2 of the
>> + * License, or (at your option) any later version
>> + */
>> +
>> +#ifndef __UAPI_MEDIA_IMX_H__
>> +#define __UAPI_MEDIA_IMX_H__
>> +
>> +/*
>> + * events from the subdevs
>> + */
>> +#define V4L2_EVENT_IMX_CLASS V4L2_EVENT_PRIVATE_START
>> +#define V4L2_EVENT_IMX_NFB4EOF (V4L2_EVENT_IMX_CLASS + 1)
>> +#define V4L2_EVENT_IMX_EOF_TIMEOUT (V4L2_EVENT_IMX_CLASS + 2)
>> +#define V4L2_EVENT_IMX_FRAME_INTERVAL (V4L2_EVENT_IMX_CLASS + 3)
> Aren't these generic enough to warrant common events? I would think
> there have to be other capture IP cores that can signal aborted frames
> or frame timeouts.
Yes, agreed. A frame capture timeout, or frame interval error, are
both generic concepts. At some point it would be great to make the
Frame Interval Monitor generally available under v4l2-core. As for the
EOF timeout event, I'll look into moving that into a generic V4L2 event.
Steve
^ permalink raw reply
* [PATCH] clk: imx6: don't restrict LDB mux changes on QuadPlus
From: Fabio Estevam @ 2017-01-13 23:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113173943.23677-1-l.stach@pengutronix.de>
On Fri, Jan 13, 2017 at 3:39 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> The LDB mux/gate layout has been fixed on QuadPlus, so there is no need
> to restrict the LDB mux changes on this hardware, as the erratum
> preventing this from working properly is gone.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
That's correct: mx6qp does not suffer from the LDB clock glitch issue:
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
^ permalink raw reply
* [PATCH 3/4] iio: adc: add a driver for the SAR ADC found in Amlogic Meson SoCs
From: Martin Blumenstingl @ 2017-01-13 23:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <055cc8fa-46c3-fab2-2d9e-be4e5416fc5a@gmail.com>
Hi Heiner,
On Fri, Jan 13, 2017 at 9:26 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
[snip]
>>>> +static int meson_saradc_read_raw_sample(struct iio_dev *indio_dev,
>>>> + const struct iio_chan_spec *chan,
>>>> + int *val)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> + int ret, regval, fifo_chan, fifo_val, sum = 0, count = 0;
>>>> +
>>>> + ret = meson_saradc_wait_busy_clear(indio_dev);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + regmap_read(priv->regmap, SAR_ADC_REG0, ®val);
>>> The resulting regval value isn't used, therefore this statement doesn't seem
>>> to be needed.
>> I can probably replace this with "0", good catch!
>>
>>> In the vendor driver there is such a dummy statement before reading the busy
>>> flags in REG0 after starting sampling. Reason seems to be a potential race
>>> when we try to read the busy flags before the sampling engine has set them.
>>> This isn't needed in meson_saradc_wait_busy_clear here as an udelay(1) is
>>> done first always.
>> do you think it's worth adding a comment here that a do ... while loop
>> is there on purpose?
>>
> Yes, a hint would be good that there's a potential race.
> Else there's a good chance that a future refactoring introduces a regression.
indeed, a comment will be part of v2
>>>> +
>>>> + while (meson_saradc_get_fifo_count(indio_dev) > 0 &&
>>> IMHO this loop isn't needed. When we come here the FIFO contains exactly
>>> one element. This is true also in averaging mode as the averaging engine
>>> writes only the resulting mean value to the FIFO.
>>>
>>> And we can't have multiple samples active in parallel due to the locking
>>> done in meson_saradc_get_sample.
>>>
>>> By the way: I use an IRQ here to wake up when the FIFO contains one
>>> element. But as you wrote: It's not clear whether this works on all
>>> Meson systems.
>> maybe I should switch to IRQ mode as even the old Meson6 vendor kernel
>> sources indicate that the SAR ADC has IRQ support?
>>
> As you like. We can also add interrupt mode later (but leave polling intact)
> and activate it only if an interrupt is set in DT.
> This way we'd have a fallback in case there should be a problem with
> interrupt mode on some system.
I'm fine with either way, my idea was to keep it simple for the start.
as a side-note: even if we get rid of polling mode we still need
something like meson_saradc_wait_busy_clear(), because it's required
to stop sampling (at least in some cases) according to the datasheet:
"To stop sampling, simply set This bit and wait for all processing
modules to no longer indicate that they are busy."
>>>> + count < SAR_ADC_MAX_FIFO_SIZE) {
>>>> + regmap_read(priv->regmap, SAR_ADC_FIFO_RD, ®val);
>>>> +
>>>> + fifo_chan = FIELD_GET(SAR_ADC_FIFO_RD_CHAN_ID_MASK, regval);
>>>> + if (fifo_chan == chan->channel) {
>>>> + fifo_val = FIELD_GET(SAR_ADC_FIFO_RD_SAMPLE_VALUE_MASK,
>>>> + regval) & SAR_ADC_VALUE_MASK(priv);
>>>> + sum += fifo_val;
>>>> + count++;
>>>> + }
>>>> + }
>>>> +
>>>> + if (!count)
>>>> + return -ENOENT;
>>>> +
>>>> + *val = sum / count;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void meson_saradc_set_averaging(struct iio_dev *indio_dev,
>>>> + const struct iio_chan_spec *chan,
>>>> + enum meson_saradc_avg_mode mode,
>>>> + enum meson_saradc_num_samples samples)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> + u32 val;
>>>> +
>>>> + val = samples << SAR_ADC_AVG_CNTL_NUM_SAMPLES_SHIFT(chan->channel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_AVG_CNTL,
>>>> + SAR_ADC_AVG_CNTL_NUM_SAMPLES_MASK(chan->channel),
>>>> + val);
>>>> +
>>>> + val = mode << SAR_ADC_AVG_CNTL_AVG_MODE_SHIFT(chan->channel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_AVG_CNTL,
>>>> + SAR_ADC_AVG_CNTL_AVG_MODE_MASK(chan->channel), val);
>>>> +}
>>>> +
>>>> +static void meson_saradc_enable_channel(struct iio_dev *indio_dev,
>>>> + const struct iio_chan_spec *chan)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> + u32 regval;
>>>> +
>>>> + /* the SAR ADC engine allows sampling multiple channels at the same
>>>> + * time. to keep it simple we're only working with one *internal*
>>>> + * channel, which starts counting at index 0 (which means: count = 1).
>>>> + */
>>>> + regval = FIELD_PREP(SAR_ADC_CHAN_LIST_MAX_INDEX_MASK, 0);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_CHAN_LIST,
>>>> + SAR_ADC_CHAN_LIST_MAX_INDEX_MASK, regval);
>>>> +
>>>> + /* map channel index 0 to the channel which we want to read */
>>>> + regval = FIELD_PREP(SAR_ADC_CHAN_CHAN_ENTRY_MASK(0), chan->channel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_CHAN_LIST,
>>>> + SAR_ADC_CHAN_CHAN_ENTRY_MASK(0), regval);
>>>> +
>>>> + regval = FIELD_PREP(SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK,
>>>> + chan->channel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_DETECT_IDLE_SW,
>>>> + SAR_ADC_DETECT_IDLE_SW_DETECT_MODE_MUX_MASK,
>>>> + regval);
>>>> +
>>>> + regval = FIELD_PREP(SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK,
>>>> + chan->channel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_DETECT_IDLE_SW,
>>>> + SAR_ADC_DETECT_IDLE_SW_IDLE_MODE_MUX_SEL_MASK,
>>>> + regval);
>>>> +
>>>> + if (chan->channel == 6)
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELTA_10,
>>>> + SAR_ADC_DELTA_10_TEMP_SEL, 0);
>>>> +}
>>>> +
>>>> +static void meson_saradc_set_channel7_mux(struct iio_dev *indio_dev,
>>>> + enum meson_saradc_chan7_mux_sel sel)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> + u32 regval;
>>>> +
>>>> + regval = FIELD_PREP(SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, sel);
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG3,
>>>> + SAR_ADC_REG3_CTRL_CHAN7_MUX_SEL_MASK, regval);
>>>> +
>>>> + usleep_range(10, 20);
>>>> +}
>>>> +
>>>> +static void meson_saradc_start_sample_engine(struct iio_dev *indio_dev)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> +
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE,
>>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE);
>>>> +
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>>> + SAR_ADC_REG0_SAMPLING_START,
>>>> + SAR_ADC_REG0_SAMPLING_START);
>>>> +}
>>>> +
>>>> +static void meson_saradc_stop_sample_engine(struct iio_dev *indio_dev)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> +
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>>> + SAR_ADC_REG0_SAMPLING_STOP,
>>>> + SAR_ADC_REG0_SAMPLING_STOP);
>>>> +
>>>> + /* wait until all modules are stopped */
>>>> + meson_saradc_wait_busy_clear(indio_dev);
>>>> +
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_REG0,
>>>> + SAR_ADC_REG0_SAMPLE_ENGINE_ENABLE, 0);
>>>> +}
>>>> +
>>>> +static void meson_saradc_lock(struct iio_dev *indio_dev)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> + int val;
>>>> +
>>>> + mutex_lock(&indio_dev->mlock);
>>>> +
>>>> + /* prevent BL30 from using the SAR ADC while we are using it */
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>>> + SAR_ADC_DELAY_KERNEL_BUSY,
>>>> + SAR_ADC_DELAY_KERNEL_BUSY);
>>>> +
>>>> + /* wait until BL30 releases it's lock (so we can use the SAR ADC) */
>>>> + do {
>>>> + udelay(1);
>>>> + regmap_read(priv->regmap, SAR_ADC_DELAY, &val);
>>>> + } while (val & SAR_ADC_DELAY_BL30_BUSY);
>>>> +}
>>>> +
>>>> +static void meson_saradc_unlock(struct iio_dev *indio_dev)
>>>> +{
>>>> + struct meson_saradc_priv *priv = iio_priv(indio_dev);
>>>> +
>>>> + /* allow BL30 to use the SAR ADC again */
>>>> + regmap_update_bits(priv->regmap, SAR_ADC_DELAY,
>>>> + SAR_ADC_DELAY_KERNEL_BUSY, 0);
>>>> +
>>>> + mutex_unlock(&indio_dev->mlock);
>>>> +}
>>>> +
>>>> +static int meson_saradc_get_sample(struct iio_dev *indio_dev,
>>>> + const struct iio_chan_spec *chan,
>>>> + enum meson_saradc_avg_mode avg_mode,
>>>> + enum meson_saradc_num_samples avg_samples,
>>>> + int *val)
>>>> +{
>>>> + int ret, tmp;
>>>> +
>>>> + meson_saradc_lock(indio_dev);
>>>> +
>>>> + /* clear old values from the FIFO buffer, ignoring errors */
>>>> + meson_saradc_read_raw_sample(indio_dev, chan, &tmp);
>>>> +
>>>> + meson_saradc_set_averaging(indio_dev, chan, avg_mode, avg_samples);
>>>> +
>>>> + meson_saradc_enable_channel(indio_dev, chan);
>>>> +
>>>> + meson_saradc_start_sample_engine(indio_dev);
>>>> + ret = meson_saradc_read_raw_sample(indio_dev, chan, val);
>>>> + meson_saradc_stop_sample_engine(indio_dev);
>>>> +
>>>> + meson_saradc_unlock(indio_dev);
>>>> +
>>>> + if (ret) {
>>>> + dev_warn(&indio_dev->dev,
>>> Using the struct device in indio_dev results in IMHO ugly messages like
>>> iio iio:device0: already initialized by BL30
>>>
>>> We should use the parent instead, this is more readable:
>>> meson-saradc c1108680.adc: already initialized by BL30
>>>
>>> For this we need to move the assignment to indio_dev->dev.parent
>>> in probe, else messages may be written when parent isn't set yet.
>> indeed, I'll change this - thanks for the hint!
a little correction on this: I am all for setting the parent early.
However, I think that devm_iio_device_alloc() itself should take care
of building the device name correctly (to prevent code-duplication
across drivers in drivers/iio/*).
as an example: phy_create() does this (which includes the OF node
name): dev_set_name(&phy->dev, "phy-%s.%d", dev_name(dev), id);
iio_device_alloc does it like this (which is obviously missing the OF
node name): dev_set_name(&dev->dev, "iio:device%d", dev->id);
Regards,
Martin
^ permalink raw reply
* [PATCH 4/4] ARM64: dts: meson: meson-gx: add the SAR ADC
From: Martin Blumenstingl @ 2017-01-13 23:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <85c07462-c175-faf8-8dae-403a6291331e@gmail.com>
On Fri, Jan 13, 2017 at 9:14 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> Am 13.01.2017 um 20:50 schrieb Martin Blumenstingl:
>> Hi Heiner,
>>
>> On Fri, Jan 13, 2017 at 8:32 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>> Sorry, I'm not subscribed to the two mailing lists, therefore my reply
>>> is outside the thread.
>>>
>>> I'm currently experimenting with an own rudimentary driver for SAR ADC
>>> on a Odroid C2 (S905GXBB). So I have some remarks based on my experience.
>> I hope that we haven't been duplicating too much work!
>>
> No, my driver doesn't include all the clock handling and relies on the
> boot loader / firmware to do this. Also I don't support averaging mode.
> Your driver is much more comprehensive and I would go with it.
>
> I have some features like interrupt mode and calibration which are not
> yet supported in your driver but they can be easily migrated and added
> later.
indeed, that'll leave some fun for you as well :-)
^ permalink raw reply
* [PATCH] ARM: BCM5301X: Set 5 GHz wireless frequency limits on Netgear R8000
From: Rafał Miłecki @ 2017-01-13 23:58 UTC (permalink / raw)
To: linux-arm-kernel
From: Rafa? Mi?ecki <rafal@milecki.pl>
Netgear R8000 is a tri-band home router. It has three BCM43602 chipsets
two of them for 5 GHz band. Both seem the same and their firmwares
report the same set of channels. The problem is due to hardware / board
design there are extra limitations that should be respected.
First PHY should be used for U-NII-2 and U-NII-3. Third PHY should be
used for U-NII-1. Using them in a different way may result in wireless
not working or in noticeably reduced performance. Basic version of this
info was provided by Broadcom employee, then it has been verified by me
using original vendor firmware (which has limitations hardcoded in UI).
This patch uses recently introduced ieee80211-freq-limit property to
describe these limitations at DT level.
Referencing PCIe devices in DT required specifying all related bridges.
Below you can see (a bit complex) PCI tree from R8000 that explains all
entries that I needed to put in DT.
0000:00:00.0 14e4:8012 Bridge Device
?? 0000:01:00.0 14e4:aa52 Network Controller
0001:00:00.0 14e4:8012 Bridge Device
?? 0001:01:00.0 10b5:8603 Bridge Device
?? 0001:02:01.0 10b5:8603 Bridge Device
? ?? 0001:03:00.0 14e4:aa52 Network Controller
?? 0001:02:02.0 10b5:8603 Bridge Device
? ?? 0001:04:00.0 14e4:aa52 Network Controller
?? 0001:02:03.0 000d:0000 0x000000
?? 0001:02:04.0 000d:0000 0x000000
?? 0001:02:05.0 000d:0000 0x000000
?? 0001:02:06.0 000d:0000 0x000000
?? (...)
?? 0001:02:1d.0 000d:0000 0x000000
?? 0001:02:1e.0 000d:0000 0x000000
?? 0001:02:1f.0 000d:0000 0x000000
Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
---
arch/arm/boot/dts/bcm4709-netgear-r8000.dts | 48 +++++++++++++++++++++++++++++
arch/arm/boot/dts/bcm5301x.dtsi | 8 +++++
2 files changed, 56 insertions(+)
diff --git a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
index cd13534..8e39a84 100644
--- a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
+++ b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
@@ -108,6 +108,54 @@
};
};
+&pcie0 {
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ bridge at 0,0,0 {
+ reg = <0x0000 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ wifi at 0,1,0 {
+ reg = <0x0000 0 0 0 0>;
+ ieee80211-freq-limit = <5735000 5835000>;
+ };
+ };
+};
+
+&pcie1 {
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ bridge at 1,0,0 {
+ reg = <0x0000 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ bridge at 1,1,0 {
+ reg = <0x0000 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ bridge at 1,2,2 {
+ reg = <0x1000 0 0 0 0>;
+
+ #address-cells = <3>;
+ #size-cells = <2>;
+
+ wifi at 1,4,0 {
+ reg = <0x0000 0 0 0 0>;
+ ieee80211-freq-limit = <5170000 5730000>;
+ };
+ };
+ };
+ };
+};
+
&usb2 {
vcc-gpio = <&chipcommon 0 GPIO_ACTIVE_HIGH>;
};
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index a4614c9..4fbb089 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -243,6 +243,14 @@
#gpio-cells = <2>;
};
+ pcie0: pcie at 12000 {
+ reg = <0x00012000 0x1000>;
+ };
+
+ pcie1: pcie at 13000 {
+ reg = <0x00013000 0x1000>;
+ };
+
usb2: usb2 at 21000 {
reg = <0x00021000 0x1000>;
--
2.10.1
^ permalink raw reply related
* [PATCH v7 00/15] ACPI platform MSI support and its example mbigen
From: Hanjun Guo @ 2017-01-14 1:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACVXFVO8n=Y+eN0Z_zMZJsirf+VNSr-y9g3uwwxQZzAxds=m4g@mail.gmail.com>
Hi Ming,
On 2017/1/13 18:23, Ming Lei wrote:
> On Wed, Jan 11, 2017 at 11:06 PM, Hanjun Guo <hanjun.guo@linaro.org> wrote:
>> With platform msi support landed in the kernel, and the introduction
>> of IORT for GICv3 ITS (PCI MSI) and SMMU, the framework for platform msi
>> is ready, this patch set add few patches to enable the ACPI platform
>> msi support.
>>
>> For platform device connecting to ITS on arm platform, we have IORT
>> table with the named componant node to describe the mappings of paltform
>> device and ITS, so we can retrieve the dev id and find its parent
>> irqdomain (ITS) from IORT table (simlar with the ACPI ITS support).
>>
>> v6 -> v7:
>> - Introduce iort_node_map_platform_id() to retrieve the
>> dev id for both NC (named component) -> ITS/SMMU and
>> NC -> SMMU -> ITS cases, suggested by Lorenzo;
>>
>> - Reorder the patches and rewrite some commit message;
>>
>> - Remove the test tags because it has major changes
>> to retrieve the dev id, Sinan, Majun, Xinwei, could
>> you please test them again on your platform?
>>
>> - rebased on top of 4.10-rc3 and Lorenzo's patch
>> https://patchwork.kernel.org/patch/9507041/
>>
>> - Tested against Agustin's patch [1-2/3] "[PATCH V9 0/3] irqchip: qcom:
>> Add IRQ combiner driver"
> Looks v7 works fine on D05, together with Lorenzo's
> fix([v2] ACPI/IORT: Fix iort_node_get_id() mapping entries indexing) and
> Agustin's patchset of "v9 irqchip: qcom: Add IRQ combiner driver".
>
> Tested-by: Ming Lei <ming.lei@canonical.com>
Thank you very much :)
Hanjun
^ permalink raw reply
* [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains
From: Rob Herring @ 2017-01-14 2:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <84d7d49b-933b-8b26-f18a-3a5054738cb1@ti.com>
On Fri, Jan 13, 2017 at 2:28 PM, Dave Gerlach <d-gerlach@ti.com> wrote:
> On 01/13/2017 01:25 PM, Rob Herring wrote:
>>
>> On Thu, Jan 12, 2017 at 9:27 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>
>>> Rob,
>>>
>>> On 01/11/2017 03:34 PM, Rob Herring wrote:
>>>>
>>>>
>>>> On Mon, Jan 9, 2017 at 11:57 AM, Dave Gerlach <d-gerlach@ti.com> wrote:
>>>>>
>>>>>
>>>>> Rob,
>>>>>
>>>>> On 01/09/2017 11:50 AM, Rob Herring wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Jan 04, 2017 at 02:55:34PM -0600, Dave Gerlach wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>>>> control device power states.
>>>>>>>
>>>>>>> Also, provide macros representing each device index as understood
>>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>>
>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>>> ---
>>>>>>> v2->v3:
>>>>>>> Update k2g_pds node docs to show it should be a child of pmmc
>>>>>>> node.
>>>>>>> In early versions a phandle was used to point to pmmc and
>>>>>>> docs
>>>>>>> still
>>>>>>> incorrectly showed this.
>>>>>>>
>>>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 59
>>>>>>> ++++++++++++++
>>>>>>> MAINTAINERS | 2 +
>>>>>>> include/dt-bindings/genpd/k2g.h | 90
>>>>>>> ++++++++++++++++++++++
>>>>>>> 3 files changed, 151 insertions(+)
>>>>>>> create mode 100644
>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..4c9064e512cb
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> @@ -0,0 +1,59 @@
>>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>>> +---------------------------------------------
>>>>>>> +
>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...)
>>>>>>> that
>>>>>>> is
>>>>>>> +responsible for controlling the state of the IPs that are present.
>>>>>>> +Communication between the host processor running an OS and the
>>>>>>> system
>>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>>>>> domain
>>>>>>> +implementation plugs into the generic pm domain framework and makes
>>>>>>> use
>>>>>>> of
>>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>>> +
>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>>> +
>>>>>>> +PM Domain Node
>>>>>>> +==============
>>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>>> PMMC,
>>>>>>> +which in this case is the single implementation as documented by the
>>>>>>> generic
>>>>>>> +PM domain bindings in
>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>>> +Because this relies on the TI SCI protocol to communicate with the
>>>>>>> PMMC
>>>>>>> it
>>>>>>> +must be a child of the pmmc node.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +--------------------
>>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>>> +- #power-domain-cells: Must be 0.
>>>>>>> +
>>>>>>> +Example (K2G):
>>>>>>> +-------------
>>>>>>> + pmmc: pmmc {
>>>>>>> + compatible = "ti,k2g-sci";
>>>>>>> + ...
>>>>>>> +
>>>>>>> + k2g_pds: k2g_pds {
>>>>>>> + compatible = "ti,sci-pm-domain";
>>>>>>> + #power-domain-cells = <0>;
>>>>>>> + };
>>>>>>> + };
>>>>>>> +
>>>>>>> +PM Domain Consumers
>>>>>>> +===================
>>>>>>> +Hardware blocks that require SCI control over their state must
>>>>>>> provide
>>>>>>> +a reference to the sci-pm-domain they are part of and a unique
>>>>>>> device
>>>>>>> +specific ID that identifies the device.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +--------------------
>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain
>>>>>>> node.
>>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI
>>>>>>> to
>>>>>>> + be used for device control.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> As I've already stated before, this goes in power-domain cells. When
>>>>>> you
>>>>>> have a single thing (i.e. node) that controls multiple things, then
>>>>>> you
>>>>>> you need to specify the ID for each of them in phandle args. This is
>>>>>> how
>>>>>> irqs, gpio, clocks, *everything* in DT works.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> You think the reasoning for doing it this way provided by both Ulf and
>>>>> myself on v2 [1] is not valid then?
>>>>>
>>>>> From Ulf:
>>>>>
>>>>> To me, the TI SCI ID, is similar to a "conid" for any another "device
>>>>> resource" (like clock, pinctrl, regulator etc) which we can describe
>>>>> in DT and assign to a device node. The only difference here, is that
>>>>> we don't have common API to fetch the resource (like clk_get(),
>>>>> regulator_get()), but instead we fetches the device's resource from
>>>>> SoC specific code, via genpd's device ->attach() callback.
>>>>
>>>>
>>>>
>>>> Sorry, but that sounds like a kernel problem to me and has nothing to
>>>> do with DT bindings.
>>>>
>>>>> From me:
>>>>>
>>>>> Yes, you've pretty much hit it on the head. It is not an index into a
>>>>> list
>>>>> of genpds but rather identifies the device *within* a single genpd. It
>>>>> is
>>>>> a
>>>>> property specific to each device that resides in a ti-sci-genpd, not a
>>>>> mapping describing which genpd the device belongs to. The generic power
>>>>> domain binding is concerned with mapping the device to a specific
>>>>> genpd,
>>>>> which is does fine for us, but we have a sub mapping for devices that
>>>>> exist
>>>>> inside a genpd which, we must describe as well, hence the ti,sci-id.
>>>>>
>>>>>
>>>>> So to summarize, the genpd framework does interpret the phandle arg as
>>>>> an
>>>>> index into multiple genpds, just as you've said other frameworks do,
>>>>> but
>>>>> this is not what I am trying to do, we have multiple devices within
>>>>> this
>>>>> *single* genpd, hence the need for the ti,sci-id property.
>>>>
>>>>
>>>>
>>>> Fix the genpd framework rather than work around it in DT.
>>>
>>>
>>>
>>> I still disagree that this has nothing to do with DT bindings, as the
>>> current DT binding represents something different already. I am trying to
>>> extend it to give me additional information needed for our platforms. Are
>>> you saying that we should break what the current DT binding already
>>> represents to mean something else?
>>
>>
>> No idea because what's the current binding? From the patch, looks like
>> a new binding to me.
>
>
> Yes, ti,sci-id is a new binding. I am referring to the current meaning of
> the "power-domains" binding, which is where you are asking this property to
> be added, in "power-domains" cells. This is documented here [1] in the
> kernel, although looking at it I must admit it is not very clear.
>
> The power-domains cell represents an offset into an array of power domains,
> if you choose to use it. That's what the genpd framework is hard coded to
> interpret it as. This is correct, as it is an index into a static list of
> power domains, used to identify which power domain a device belongs to,
> which is exactly what the genpd framework itself is concerned with. This is
> already how it is used in the kernel today.
Strictly speaking, the cells are purely for the interpretation of the
phandle they are associated with. If some controller wants to have 20
cells, then it could assuming a good reason. The reality is we tend to
align the meaning of the cells. If genpd is interpreting the cells and
not letting the driver for the power domain controller interpret them,
then still, genpd needs to be fixed.
IIRC, initially it was said genpd required 0 cells, hence my confusion.
> My ti,sci-id is not an index into a list of power domains, so it should not
> go in the power-domains cells and go against what the power-domains binding
> says that the cell expects. We have one single power domain, and the new
> ti,sci-id binding is not something the genpd framework itself is concerned
> with as it's our property to identify a device inside a power domain, not to
> identify which power domain it is associated with.
What is the id used for? I can understand why you need to know what
power domain a device is in (as power-domains identifies), but not
what devices are in a power domain.
Rob
^ permalink raw reply
* [PATCH v2 1/2] of: base: add support to find the level of the last cache
From: Rob Herring @ 2017-01-14 2:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484245772-31511-1-git-send-email-sudeep.holla@arm.com>
On Thu, Jan 12, 2017 at 12:29 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> It is useful to have helper function just to get the number of cache
> levels for a given logical cpu. We can obtain the same by just checking
> the level at which the last cache is present. This patch adds support
> to find the level of the last cache for a given cpu.
>
> It will be used on ARM64 platform where the device tree provides the
> information for the additional non-architected/transparent/external
> last level caches that are not integrated with the processors.
>
> Suggested-by: Rob Herring <robh+dt@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/of/base.c | 27 +++++++++++++++++++++++++++
> include/linux/of.h | 1 +
> 2 files changed, 28 insertions(+)
>
> v1->v2:
> - Moved to using "cache-level" in the last level cache instead
> of counting through all the nodes as suggested by Rob
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d4bea3c797d6..c1128a077aea 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -25,6 +25,7 @@
> #include <linux/cpu.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/of_graph.h>
> #include <linux/spinlock.h>
> #include <linux/slab.h>
> @@ -2268,6 +2269,32 @@ struct device_node *of_find_next_cache_node(const struct device_node *np)
> }
>
> /**
> + * of_find_last_cache_level - Find the level at which the last cache is
> + * present for the given logical cpu
> + *
> + * @cpu: cpu number(logical index) for which the last cache level is needed
> + *
> + * Returns the the level at which the last cache is present. It is exactly
> + * same as the total number of cache levels for the given logical cpu.
> + */
> +int of_find_last_cache_level(unsigned int cpu)
> +{
> + int cache_level = 0;
> + struct device_node *prev = NULL, *np = of_cpu_device_node_get(cpu);
> +
> + while (np) {
> + prev = np;
> + of_node_put(np);
> + np = of_find_next_cache_node(np);
> + }
> +
> + if (prev)
Probably don't need this check. Otherwise,
Acked-by: Rob Herring <robh@kernel.org>
> + of_property_read_u32(prev, "cache-level", &cache_level);
> +
> + return cache_level;
> +}
> +
> +/**
> * of_graph_parse_endpoint() - parse common endpoint node properties
> * @node: pointer to endpoint device_node
> * @endpoint: pointer to the OF endpoint data structure
> diff --git a/include/linux/of.h b/include/linux/of.h
> index d72f01009297..21e6323de0f3 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -280,6 +280,7 @@ extern struct device_node *of_get_child_by_name(const struct device_node *node,
>
> /* cache lookup */
> extern struct device_node *of_find_next_cache_node(const struct device_node *);
> +extern int of_find_last_cache_level(unsigned int cpu);
> extern struct device_node *of_find_node_with_property(
> struct device_node *from, const char *prop_name);
>
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v7 15/15] irqchip: mbigen: Add ACPI support
From: Hanjun Guo @ 2017-01-14 2:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113102104.GB20837@red-moon>
Hi Lorenzo,
On 2017/1/13 18:21, Lorenzo Pieralisi wrote:
> On Wed, Jan 11, 2017 at 11:06:39PM +0800, Hanjun Guo wrote:
>> With the preparation of platform msi support and interrupt producer
>> in DSDT, we can add mbigen ACPI support now.
>>
>> We are using _PRS methd to indicate number of irq pins instead
>> of num_pins in DT to avoid _DSD usage in this case.
>>
>> For mbi-gen,
>> Device(MBI0) {
>> Name(_HID, "HISI0152")
>> Name(_UID, Zero)
>> Name(_CRS, ResourceTemplate() {
>> Memory32Fixed(ReadWrite, 0xa0080000, 0x10000)
>> })
>>
>> Name (_PRS, ResourceTemplate() {
>> Interrupt(ResourceProducer,...) {12,14,....}
> I still do not understand why you are using _PRS for this, I think
> the MBIgen configuration is static and if it is so the Interrupt
> resource should be part of the _CRS unless there is something I am
> missing here.
Sorry for not clear in the commit message. MBIgen is an interrupt producer
which produces irq resource to devices connecting to it, and MBIgen itself
don't consume wired interrupts.
Also devices connecting MBIgen may not consume all the interrupts produced
by MBIgen, for example, MBIgen may produce 128 interrupts but only half of
them are currently used, so _PRS here means "provide interrupt resources
may consumed by devices connecting to it".
Should I add this into the commit message?
Thanks
Hanjun
^ permalink raw reply
* [PATCH v7 12/15] msi: platform: make platform_msi_create_device_domain() ACPI aware
From: Hanjun Guo @ 2017-01-14 3:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113104524.GC20837@red-moon>
On 2017/1/13 18:45, Lorenzo Pieralisi wrote:
> On Wed, Jan 11, 2017 at 11:06:36PM +0800, Hanjun Guo wrote:
>> platform_msi_create_device_domain() is used to ctreate
>> irqdomain for the device such as irqchip mbigen generating
>> the MSIs, it's almost ready for ACPI use except
>> of_node_to_fwnode() is for dt only, make it ACPI aware then
>> things will work in both DTS and ACPI.
> "The irqdomain creation carried out in:
>
> platform_msi_create_device_domain()
>
> relies on the fwnode_handle interrupt controller token to associate the
> interrupt controller with a specific irqdomain. Current code relies on
> the OF layer to retrieve a fwnode_handle for the device representing the
> interrupt controller from its device->of_node pointer. This makes
> platform_msi_create_device_domain() DT specific whilst it really is not
> because after the merge of commit f94277af03ea ("of/platform: Initialise
> dev->fwnode appropriately") the fwnode_handle can easily be retrieved
> from the dev->fwnode pointer in a firmware agnostic way.
>
> Update platform_msi_create_device_domain() to retrieve the interrupt
> controller fwnode_handle from the dev->fwnode pointer so that it can
> be used seamlessly in ACPI and DT systems".
Much better, I will update the patch.
>
> Reviewed-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Thanks
Hanjun
^ permalink raw reply
* [PATCH v7 08/15] ACPI: IORT: rename iort_node_map_rid() to make it generic
From: Hanjun Guo @ 2017-01-14 3:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113114747.GD20837@red-moon>
On 2017/1/13 19:47, Lorenzo Pieralisi wrote:
> On Wed, Jan 11, 2017 at 11:06:32PM +0800, Hanjun Guo wrote:
>> iort_node_map_rid() was designed for both PCI and platform
>> device, but the rid means requester id is for ITS mappings,
> I do not understand what this means sorry.
>
>> rename iort_node_map_rid() to iort_node_map_id() and update
>> its argument names to make it more generic.
>>
> "iort_node_map_rid() was designed to take an input id (that is not
> necessarily a PCI requester id) and map it to an output id (eg an SMMU
> streamid or an ITS deviceid) according to the mappings provided by an
> IORT node mapping entries. This means that the iort_node_map_rid() input
> id is not always a PCI requester id as its name, parameters and local
> variables suggest, which is misleading.
>
> Apply the s/rid/id substitution to the iort_node_map_rid() mapping
> function and its users to make sure its intended usage is clearer."
Thank your patience, I will update the commit message.
Hanjun
^ permalink raw reply
* [PATCH v7 09/15] ACPI: platform-msi: retrieve dev id from IORT
From: Hanjun Guo @ 2017-01-14 4:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170113121144.GE20837@red-moon>
Hi Lorenzo,
On 2017/1/13 20:11, Lorenzo Pieralisi wrote:
> On Wed, Jan 11, 2017 at 11:06:33PM +0800, Hanjun Guo wrote:
>> For devices connecting to ITS, it needs dev id to identify itself, and
>> this dev id is represented in the IORT table in named component node
>> [1] for platform devices, so in this patch we will scan the IORT to
>> retrieve device's dev id.
>>
>> For named components we know that there are always two steps
>> involved (second optional):
>>
>> (1) Retrieve the initial id (this may well provide the final mapping)
>> (2) Map the id (optional if (1) represents the map type we need), this
>> is needed for use cases such as NC (named component) -> SMMU -> ITS
>> mappings.
>>
>> we have API iort_node_get_id() for step (1) above and
>> iort_node_map_rid() for step (2), so create a wrapper
>> iort_node_map_platform_id() to retrieve the dev id.
>>
>> [1]: https://static.docs.arm.com/den0049/b/DEN0049B_IO_Remapping_Table.pdf
> This patch should be split and IORT changes should be squashed with
> patch 10.
If split the changes for IORT and its platform msi, API introduced in IORT will
not be used in a single patch, seems violate the suggestion of "new introduced API
needs to be used in the same patch", did I miss something?
>
>> Suggested-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Suggested-by: Tomasz Nowicki <tn@semihalf.com>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Sinan Kaya <okaya@codeaurora.org>
>> Cc: Tomasz Nowicki <tn@semihalf.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> ---
>> drivers/acpi/arm64/iort.c | 56 +++++++++++++++++++++++++++
>> drivers/irqchip/irq-gic-v3-its-platform-msi.c | 4 +-
>> include/linux/acpi_iort.h | 8 ++++
>> 3 files changed, 67 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>> index 069a690..95fd20b 100644
>> --- a/drivers/acpi/arm64/iort.c
>> +++ b/drivers/acpi/arm64/iort.c
>> @@ -30,6 +30,7 @@
>> #define IORT_MSI_TYPE (1 << ACPI_IORT_NODE_ITS_GROUP)
>> #define IORT_IOMMU_TYPE ((1 << ACPI_IORT_NODE_SMMU) | \
>> (1 << ACPI_IORT_NODE_SMMU_V3))
>> +#define IORT_TYPE_ANY (IORT_MSI_TYPE | IORT_IOMMU_TYPE)
>>
>> struct iort_its_msi_chip {
>> struct list_head list;
>> @@ -406,6 +407,34 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>> return NULL;
>> }
>>
>> +static
>> +struct acpi_iort_node *iort_node_map_platform_id(struct acpi_iort_node *node,
>> + u32 *id_out, u8 type_mask,
>> + int index)
>> +{
>> + struct acpi_iort_node *parent;
>> + u32 id;
>> +
>> + /* step 1: retrieve the initial dev id */
>> + parent = iort_node_get_id(node, &id, IORT_TYPE_ANY, index);
>> + if (!parent)
>> + return NULL;
>> +
>> + /*
>> + * optional step 2: map the initial dev id if its parent is not
>> + * the target type we wanted, map it again for the use cases such
>> + * as NC (named component) -> SMMU -> ITS. If the type is matched,
>> + * return the parent pointer directly.
>> + */
>> + if (!(IORT_TYPE_MASK(parent->type) & type_mask))
>> + parent = iort_node_map_id(parent, id, id_out, type_mask);
>> + else
>> + if (id_out)
> Remove this pointer check.
This was added because of NULL pointer reference, I passed NULL for id_out because I
only want to get its parent node, I think we have four options:
- Introduce a new API to get the parent only from the scratch, but it will duplicate the code
a lot;
- Don't check the id_out in iort_node_map_platform_id(), and introduce a wrapper and pass the
dummy id for iort_node_map_platform_id() :
static
struct acpi_iort_node *iort_node_get_platform_parent{struct device *dev, u8 type_mask}
{
struct acpi_iort_node *node, *parent = NULL;
int i;
u32 dummy_id;
node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
iort_match_node_callback, dev);
if (!node)
return NULL;
for (i = 0; i < node->mapping_count; i++) {
/* we just want to get the parent node */
parent = iort_node_map_platform_id(node, &dummy_id,
IORT_MSI_TYPE, i);
if (parent)
break;
}
return parent;
}
- Similar solution as above but don't introduce wrapper, just use dummy_id if
iort_node_map_platform_id() is called;
- Use the solution I proposed in this patch.
Please share you suggestion on this :)
>
>> + *id_out = id;
>> +
>> + return parent;
>> +}
>> +
>> static struct acpi_iort_node *iort_find_dev_node(struct device *dev)
>> {
>> struct pci_bus *pbus;
>> @@ -444,6 +473,33 @@ u32 iort_msi_map_rid(struct device *dev, u32 req_id)
>> }
>>
>> /**
>> + * iort_pmsi_get_dev_id() - Get the device id for a device
>> + * @dev: The device for which the mapping is to be done.
>> + * @dev_id: The device ID found.
>> + *
>> + * Returns: 0 for successful find a dev id, errors otherwise
> Nit: -ENODEV on error
>
>> + */
>> +int iort_pmsi_get_dev_id(struct device *dev, u32 *dev_id)
>> +{
>> + int i;
>> + struct acpi_iort_node *node;
>> +
>> + if (!iort_table)
>> + return -ENODEV;
> I do not think this iort_table check is needed.
Agreed, it will be checked in iort_scan_node() and it's called
in iort_find_dev_node().
>
>> + node = iort_find_dev_node(dev);
>> + if (!node)
>> + return -ENODEV;
>> +
>> + for (i = 0; i < node->mapping_count; i++) {
>> + if(iort_node_map_platform_id(node, dev_id, IORT_MSI_TYPE, i))
> ^
>
> Nit: Missing a space.
I was on a flight when updating the patches, seems it's not a good place for coding :)
I will update the patch set when you are ok with the solutions I proposed, thank you
very much for the review.
Hanjun
^ permalink raw reply
* [PATCH v7 00/15] ACPI platform MSI support and its example mbigen
From: Hanjun Guo @ 2017-01-14 4:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5878E02E.5080602@hisilicon.com>
Hi Wei,
On 2017/1/13 22:11, Wei Xu wrote:
> Hi Hanjun,
>
> On 2017/1/11 15:06, Hanjun Guo wrote:
>> With platform msi support landed in the kernel, and the introduction
>> of IORT for GICv3 ITS (PCI MSI) and SMMU, the framework for platform msi
>> is ready, this patch set add few patches to enable the ACPI platform
>> msi support.
>>
>> For platform device connecting to ITS on arm platform, we have IORT
>> table with the named componant node to describe the mappings of paltform
>> device and ITS, so we can retrieve the dev id and find its parent
>> irqdomain (ITS) from IORT table (simlar with the ACPI ITS support).
>>
>> v6 -> v7:
>> - Introduce iort_node_map_platform_id() to retrieve the
>> dev id for both NC (named component) -> ITS/SMMU and
>> NC -> SMMU -> ITS cases, suggested by Lorenzo;
>>
>> - Reorder the patches and rewrite some commit message;
>>
>> - Remove the test tags because it has major changes
>> to retrieve the dev id, Sinan, Majun, Xinwei, could
>> you please test them again on your platform?
>>
>> - rebased on top of 4.10-rc3 and Lorenzo's patch
>> https://patchwork.kernel.org/patch/9507041/
>>
>> - Tested against Agustin's patch [1-2/3] "[PATCH V9 0/3] irqchip: qcom:
>> Add IRQ combiner driver"
> I tested this patch set on the Hisilicon D05 board with these patches:
>
> [1] Agustin's V9 IRQ combiner driver patch set
> https://patchwork.kernel.org/patch/9474751/
>
> [2] Lorenzo's v2 iort_node_get_id fix patch
> https://patchwork.kernel.org/patch/9507041/
>
> The branch is at https://github.com/hisilicon/linux-hisi/tree/topic-acpi-mbigen.
>
> The integrated XGE, SAS and PCIe controller works fine.
> So with this patch set:
>
> Tested-by: Wei Xu <xuwei5@hisilicon.com>
I didn't test PCIe it's good to know it's working, thanks for testing
and let me know.
Hanjun
^ permalink raw reply
* [PATCH v6 0/3] dmaengine: xilinx_dma: Bug fixes
From: Kedareswara rao Appana @ 2017-01-14 5:35 UTC (permalink / raw)
To: linux-arm-kernel
This patch series fixes below bugs in DMA and VDMA IP's
---> Do not start VDMA until frame buffer is processed by the h/w
---> Fix bug in Multi frame sotres handling in VDMA
---> Fix issues w.r.to multi frame descriptors submit with AXI DMA S2MM(recv) Side.
Kedareswara rao Appana (3):
dmaengine: xilinx_dma: Check for channel idle state before submitting
dma descriptor
dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in
vdma
dmaengine: xilinx_dma: Fix race condition in the driver for multiple
descriptor scenario
.../devicetree/bindings/dma/xilinx/xilinx_dma.txt | 2 +
drivers/dma/xilinx/xilinx_dma.c | 270 ++++++++++++---------
2 files changed, 161 insertions(+), 111 deletions(-)
--
2.1.2
^ permalink raw reply
* [PATCH v6 1/3] dmaengine: xilinx_dma: Check for channel idle state before submitting dma descriptor
From: Kedareswara rao Appana @ 2017-01-14 5:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484372155-19423-1-git-send-email-appanad@xilinx.com>
Add variable for checking channel idle state to ensure that dma descriptor is not
Submitted when DMA engine is in progress.
This will avoids the pollling for a bit in the status register to know
Dma state in the driver hot path.
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v6:
---> Updated commit message as suggested by Vinod.
---> Added Channel idle variable description in the driver
as suggested by Vinod.
Changes for v5:
---> None.
Changes for v4:
---> None.
Changes for v3:
---> None.
Changes for v2:
---> Add idle check in the reset as suggested by Jose Abreu
---> Removed xilinx_dma_is_running/xilinx_dma_is_idle checks
in the driver and used common idle checks across the driver
as suggested by Laurent Pinchart.
drivers/dma/xilinx/xilinx_dma.c | 61 +++++++++++++++--------------------------
1 file changed, 22 insertions(+), 39 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 8288fe4..5eeea57 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -321,6 +321,7 @@ struct xilinx_dma_tx_descriptor {
* @cyclic: Check for cyclic transfers.
* @genlock: Support genlock mode
* @err: Channel has errors
+ * @idle: Check for channel idle
* @tasklet: Cleanup work after irq
* @config: Device configuration info
* @flush_on_fsync: Flush on Frame sync
@@ -351,6 +352,7 @@ struct xilinx_dma_chan {
bool cyclic;
bool genlock;
bool err;
+ bool idle;
struct tasklet_struct tasklet;
struct xilinx_vdma_config config;
bool flush_on_fsync;
@@ -920,32 +922,6 @@ static enum dma_status xilinx_dma_tx_status(struct dma_chan *dchan,
}
/**
- * xilinx_dma_is_running - Check if DMA channel is running
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if running, '0' if not.
- */
-static bool xilinx_dma_is_running(struct xilinx_dma_chan *chan)
-{
- return !(dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
- XILINX_DMA_DMASR_HALTED) &&
- (dma_ctrl_read(chan, XILINX_DMA_REG_DMACR) &
- XILINX_DMA_DMACR_RUNSTOP);
-}
-
-/**
- * xilinx_dma_is_idle - Check if DMA channel is idle
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if idle, '0' if not.
- */
-static bool xilinx_dma_is_idle(struct xilinx_dma_chan *chan)
-{
- return dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
- XILINX_DMA_DMASR_IDLE;
-}
-
-/**
* xilinx_dma_halt - Halt DMA channel
* @chan: Driver specific DMA channel
*/
@@ -966,6 +942,7 @@ static void xilinx_dma_halt(struct xilinx_dma_chan *chan)
chan, dma_ctrl_read(chan, XILINX_DMA_REG_DMASR));
chan->err = true;
}
+ chan->idle = true;
}
/**
@@ -1007,6 +984,9 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
if (chan->err)
return;
+ if (!chan->idle)
+ return;
+
if (list_empty(&chan->pending_list))
return;
@@ -1018,13 +998,6 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
tail_segment = list_last_entry(&tail_desc->segments,
struct xilinx_vdma_tx_segment, node);
- /* If it is SG mode and hardware is busy, cannot submit */
- if (chan->has_sg && xilinx_dma_is_running(chan) &&
- !xilinx_dma_is_idle(chan)) {
- dev_dbg(chan->dev, "DMA controller still busy\n");
- return;
- }
-
/*
* If hardware is idle, then all descriptors on the running lists are
* done, start new transfers
@@ -1110,6 +1083,7 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, last->hw.vsize);
}
+ chan->idle = false;
if (!chan->has_sg) {
list_del(&desc->node);
list_add_tail(&desc->node, &chan->active_list);
@@ -1136,6 +1110,9 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
if (chan->err)
return;
+ if (!chan->idle)
+ return;
+
if (list_empty(&chan->pending_list))
return;
@@ -1181,6 +1158,7 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
list_splice_tail_init(&chan->pending_list, &chan->active_list);
chan->desc_pendingcount = 0;
+ chan->idle = false;
}
/**
@@ -1196,15 +1174,11 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
if (chan->err)
return;
- if (list_empty(&chan->pending_list))
+ if (!chan->idle)
return;
- /* If it is SG mode and hardware is busy, cannot submit */
- if (chan->has_sg && xilinx_dma_is_running(chan) &&
- !xilinx_dma_is_idle(chan)) {
- dev_dbg(chan->dev, "DMA controller still busy\n");
+ if (list_empty(&chan->pending_list))
return;
- }
head_desc = list_first_entry(&chan->pending_list,
struct xilinx_dma_tx_descriptor, node);
@@ -1302,6 +1276,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
list_splice_tail_init(&chan->pending_list, &chan->active_list);
chan->desc_pendingcount = 0;
+ chan->idle = false;
}
/**
@@ -1366,6 +1341,7 @@ static int xilinx_dma_reset(struct xilinx_dma_chan *chan)
}
chan->err = false;
+ chan->idle = true;
return err;
}
@@ -1447,6 +1423,7 @@ static irqreturn_t xilinx_dma_irq_handler(int irq, void *data)
if (status & XILINX_DMA_DMASR_FRM_CNT_IRQ) {
spin_lock(&chan->lock);
xilinx_dma_complete_descriptor(chan);
+ chan->idle = true;
chan->start_transfer(chan);
spin_unlock(&chan->lock);
}
@@ -2327,6 +2304,12 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
chan->has_sg = xdev->has_sg;
chan->desc_pendingcount = 0x0;
chan->ext_addr = xdev->ext_addr;
+ /* This variable enusres that descripotrs are not
+ * Submited when dma engine is in progress. This variable is
+ * Added to avoid pollling for a bit in the status register to
+ * Know dma state in the driver hot path.
+ */
+ chan->idle = true;
spin_lock_init(&chan->lock);
INIT_LIST_HEAD(&chan->pending_list);
--
2.1.2
^ permalink raw reply related
* [PATCH v6 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma
From: Kedareswara rao Appana @ 2017-01-14 5:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484372155-19423-1-git-send-email-appanad@xilinx.com>
When VDMA is configured for more than one frame in the h/w.
For example h/w is configured for n number of frames, user
Submits n number of frames and triggered the DMA using issue_pending API.
In the current driver flow we are submitting one frame at a time,
But we should submit all the n number of frames at one time
As the h/w is configured for n number of frames.
This patch fixes this issue.
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v6:
---> Added Rob Acked-by
---> Updated commit message as suggested by Vinod.
Changes for v5:
---> Updated xlnx,fstore-config property to xlnx,fstore-enable
and updated description as suggested by Rob.
Changes for v4:
---> Add Check for framestore configuration on Transmit case as well
as suggested by Jose Abreu.
---> Modified the dev_dbg checks to dev_warn checks as suggested
by Jose Abreu.
Changes for v3:
---> Added Checks for frame store configuration. If frame store
Configuration is not present at the h/w level and user
Submits less frames added debug prints in the driver as relevant.
Changes for v2:
---> Fixed race conditions in the driver as suggested by Jose Abreu
---> Fixed unnecessray if else checks in the vdma_start_transfer
as suggested by Laurent Pinchart.
.../devicetree/bindings/dma/xilinx/xilinx_dma.txt | 2 +
drivers/dma/xilinx/xilinx_dma.c | 78 +++++++++++++++-------
2 files changed, 57 insertions(+), 23 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
index a2b8bfa..e951c09 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
@@ -66,6 +66,8 @@ Optional child node properties:
Optional child node properties for VDMA:
- xlnx,genlock-mode: Tells Genlock synchronization is
enabled/disabled in hardware.
+- xlnx,fstore-enable: boolean; if defined, it indicates that controller
+ supports frame store configuration.
Optional child node properties for AXI DMA:
-dma-channels: Number of dma channels in child node.
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 5eeea57..edb5b71 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -322,6 +322,7 @@ struct xilinx_dma_tx_descriptor {
* @genlock: Support genlock mode
* @err: Channel has errors
* @idle: Check for channel idle
+ * @has_fstoreen: Check for frame store configuration
* @tasklet: Cleanup work after irq
* @config: Device configuration info
* @flush_on_fsync: Flush on Frame sync
@@ -353,6 +354,7 @@ struct xilinx_dma_chan {
bool genlock;
bool err;
bool idle;
+ bool has_fstoreen;
struct tasklet_struct tasklet;
struct xilinx_vdma_config config;
bool flush_on_fsync;
@@ -990,6 +992,27 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
if (list_empty(&chan->pending_list))
return;
+ /*
+ * Note: When VDMA is built with default h/w configuration
+ * User should submit frames upto H/W configured.
+ * If users submits less than h/w configured
+ * VDMA engine tries to write to a invalid location
+ * Results undefined behaviour/memory corruption.
+ *
+ * If user would like to submit frames less than h/w capable
+ * On S2MM side please enable debug info 13 at the h/w level
+ * On MM2S side please enable debug info 6 at the h/w level
+ * It will allows the frame buffers numbers to be modified at runtime.
+ */
+ if (!chan->has_fstoreen &&
+ chan->desc_pendingcount < chan->num_frms) {
+ dev_warn(chan->dev, "Frame Store Configuration is not enabled at the\n");
+ dev_warn(chan->dev, "H/w level enable Debug info 13 or 6 at the h/w level\n");
+ dev_warn(chan->dev, "OR Submit the frames upto h/w Capable\n\r");
+
+ return;
+ }
+
desc = list_first_entry(&chan->pending_list,
struct xilinx_dma_tx_descriptor, node);
tail_desc = list_last_entry(&chan->pending_list,
@@ -1052,25 +1075,38 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
if (chan->has_sg) {
dma_ctrl_write(chan, XILINX_DMA_REG_TAILDESC,
tail_segment->phys);
+ list_splice_tail_init(&chan->pending_list, &chan->active_list);
+ chan->desc_pendingcount = 0;
} else {
struct xilinx_vdma_tx_segment *segment, *last = NULL;
- int i = 0;
+ int i = 0, j = 0;
if (chan->desc_submitcount < chan->num_frms)
i = chan->desc_submitcount;
- list_for_each_entry(segment, &desc->segments, node) {
- if (chan->ext_addr)
- vdma_desc_write_64(chan,
- XILINX_VDMA_REG_START_ADDRESS_64(i++),
- segment->hw.buf_addr,
- segment->hw.buf_addr_msb);
- else
- vdma_desc_write(chan,
- XILINX_VDMA_REG_START_ADDRESS(i++),
- segment->hw.buf_addr);
-
- last = segment;
+ for (j = 0; j < chan->num_frms; ) {
+ list_for_each_entry(segment, &desc->segments, node) {
+ if (chan->ext_addr)
+ vdma_desc_write_64(chan,
+ XILINX_VDMA_REG_START_ADDRESS_64(i++),
+ segment->hw.buf_addr,
+ segment->hw.buf_addr_msb);
+ else
+ vdma_desc_write(chan,
+ XILINX_VDMA_REG_START_ADDRESS(i++),
+ segment->hw.buf_addr);
+
+ last = segment;
+ }
+ list_del(&desc->node);
+ list_add_tail(&desc->node, &chan->active_list);
+ j++;
+ if (list_empty(&chan->pending_list) ||
+ (i == chan->num_frms))
+ break;
+ desc = list_first_entry(&chan->pending_list,
+ struct xilinx_dma_tx_descriptor,
+ node);
}
if (!last)
@@ -1081,20 +1117,14 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
vdma_desc_write(chan, XILINX_DMA_REG_FRMDLY_STRIDE,
last->hw.stride);
vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, last->hw.vsize);
- }
- chan->idle = false;
- if (!chan->has_sg) {
- list_del(&desc->node);
- list_add_tail(&desc->node, &chan->active_list);
- chan->desc_submitcount++;
- chan->desc_pendingcount--;
+ chan->desc_submitcount += j;
+ chan->desc_pendingcount -= j;
if (chan->desc_submitcount == chan->num_frms)
chan->desc_submitcount = 0;
- } else {
- list_splice_tail_init(&chan->pending_list, &chan->active_list);
- chan->desc_pendingcount = 0;
}
+
+ chan->idle = false;
}
/**
@@ -1342,6 +1372,7 @@ static int xilinx_dma_reset(struct xilinx_dma_chan *chan)
chan->err = false;
chan->idle = true;
+ chan->desc_submitcount = 0;
return err;
}
@@ -2320,6 +2351,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
has_dre = of_property_read_bool(node, "xlnx,include-dre");
chan->genlock = of_property_read_bool(node, "xlnx,genlock-mode");
+ chan->has_fstoreen = of_property_read_bool(node, "xlnx,fstore-enable");
err = of_property_read_u32(node, "xlnx,datawidth", &value);
if (err) {
--
2.1.2
^ permalink raw reply related
* [PATCH v6 3/3] dmaengine: xilinx_dma: Fix race condition in the driver for multiple descriptor scenario
From: Kedareswara rao Appana @ 2017-01-14 5:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484372155-19423-1-git-send-email-appanad@xilinx.com>
As per AXI DMA spec the software must not move the tail pointer to a location
That has not been updated (next descriptor field of the h/w descriptor
Should always point to a valid address).
When user submits multiple descriptors on the recv side, with the
Current driver flow the last buffer descriptor next descriptor field
Points to a invalid location, resulting the invalid data or errors in the
DMA engine.
This patch fixes this issue by creating a Buffer Descritpor Chain during
Channel allocation itself and use those Buffer Descriptors.
Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v6:
---> Updated Commit message as suggested by Vinod.
Changes for v5:
---> None.
Changes for v4:
---> None.
Changes for v3:
---> None.
Changes for v2:
---> None.
drivers/dma/xilinx/xilinx_dma.c | 133 +++++++++++++++++++++++++---------------
1 file changed, 83 insertions(+), 50 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index edb5b71..c5cd935 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -163,6 +163,7 @@
#define XILINX_DMA_BD_SOP BIT(27)
#define XILINX_DMA_BD_EOP BIT(26)
#define XILINX_DMA_COALESCE_MAX 255
+#define XILINX_DMA_NUM_DESCS 255
#define XILINX_DMA_NUM_APP_WORDS 5
/* Multi-Channel DMA Descriptor offsets*/
@@ -310,6 +311,7 @@ struct xilinx_dma_tx_descriptor {
* @pending_list: Descriptors waiting
* @active_list: Descriptors ready to submit
* @done_list: Complete descriptors
+ * @free_seg_list: Free descriptors
* @common: DMA common channel
* @desc_pool: Descriptors pool
* @dev: The dma device
@@ -331,7 +333,9 @@ struct xilinx_dma_tx_descriptor {
* @desc_submitcount: Descriptor h/w submitted count
* @residue: Residue for AXI DMA
* @seg_v: Statically allocated segments base
+ * @seg_p: Physical allocated segments base
* @cyclic_seg_v: Statically allocated segment base for cyclic transfers
+ * @cyclic_seg_p: Physical allocated segments base for cyclic dma
* @start_transfer: Differentiate b/w DMA IP's transfer
*/
struct xilinx_dma_chan {
@@ -342,6 +346,7 @@ struct xilinx_dma_chan {
struct list_head pending_list;
struct list_head active_list;
struct list_head done_list;
+ struct list_head free_seg_list;
struct dma_chan common;
struct dma_pool *desc_pool;
struct device *dev;
@@ -363,7 +368,9 @@ struct xilinx_dma_chan {
u32 desc_submitcount;
u32 residue;
struct xilinx_axidma_tx_segment *seg_v;
+ dma_addr_t seg_p;
struct xilinx_axidma_tx_segment *cyclic_seg_v;
+ dma_addr_t cyclic_seg_p;
void (*start_transfer)(struct xilinx_dma_chan *chan);
u16 tdest;
};
@@ -569,17 +576,31 @@ static struct xilinx_axidma_tx_segment *
xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan)
{
struct xilinx_axidma_tx_segment *segment;
- dma_addr_t phys;
-
- segment = dma_pool_zalloc(chan->desc_pool, GFP_ATOMIC, &phys);
- if (!segment)
- return NULL;
+ unsigned long flags;
- segment->phys = phys;
+ spin_lock_irqsave(&chan->lock, flags);
+ if (!list_empty(&chan->free_seg_list)) {
+ segment = list_first_entry(&chan->free_seg_list,
+ struct xilinx_axidma_tx_segment,
+ node);
+ list_del(&segment->node);
+ }
+ spin_unlock_irqrestore(&chan->lock, flags);
return segment;
}
+static void xilinx_dma_clean_hw_desc(struct xilinx_axidma_desc_hw *hw)
+{
+ u32 next_desc = hw->next_desc;
+ u32 next_desc_msb = hw->next_desc_msb;
+
+ memset(hw, 0, sizeof(struct xilinx_axidma_desc_hw));
+
+ hw->next_desc = next_desc;
+ hw->next_desc_msb = next_desc_msb;
+}
+
/**
* xilinx_dma_free_tx_segment - Free transaction segment
* @chan: Driver specific DMA channel
@@ -588,7 +609,9 @@ xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan)
static void xilinx_dma_free_tx_segment(struct xilinx_dma_chan *chan,
struct xilinx_axidma_tx_segment *segment)
{
- dma_pool_free(chan->desc_pool, segment, segment->phys);
+ xilinx_dma_clean_hw_desc(&segment->hw);
+
+ list_add_tail(&segment->node, &chan->free_seg_list);
}
/**
@@ -713,16 +736,26 @@ static void xilinx_dma_free_descriptors(struct xilinx_dma_chan *chan)
static void xilinx_dma_free_chan_resources(struct dma_chan *dchan)
{
struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
+ unsigned long flags;
dev_dbg(chan->dev, "Free all channel resources.\n");
xilinx_dma_free_descriptors(chan);
+
if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
- xilinx_dma_free_tx_segment(chan, chan->cyclic_seg_v);
- xilinx_dma_free_tx_segment(chan, chan->seg_v);
+ spin_lock_irqsave(&chan->lock, flags);
+ INIT_LIST_HEAD(&chan->free_seg_list);
+ spin_unlock_irqrestore(&chan->lock, flags);
+
+ /* Free Memory that is allocated for cyclic DMA Mode */
+ dma_free_coherent(chan->dev, sizeof(*chan->cyclic_seg_v),
+ chan->cyclic_seg_v, chan->cyclic_seg_p);
+ }
+
+ if (chan->xdev->dma_config->dmatype != XDMA_TYPE_AXIDMA) {
+ dma_pool_destroy(chan->desc_pool);
+ chan->desc_pool = NULL;
}
- dma_pool_destroy(chan->desc_pool);
- chan->desc_pool = NULL;
}
/**
@@ -805,6 +838,7 @@ static void xilinx_dma_do_tasklet(unsigned long data)
static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
{
struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
+ int i;
/* Has this channel already been allocated? */
if (chan->desc_pool)
@@ -815,11 +849,30 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
* for meeting Xilinx VDMA specification requirement.
*/
if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
- chan->desc_pool = dma_pool_create("xilinx_dma_desc_pool",
- chan->dev,
- sizeof(struct xilinx_axidma_tx_segment),
- __alignof__(struct xilinx_axidma_tx_segment),
- 0);
+ /* Allocate the buffer descriptors. */
+ chan->seg_v = dma_zalloc_coherent(chan->dev,
+ sizeof(*chan->seg_v) *
+ XILINX_DMA_NUM_DESCS,
+ &chan->seg_p, GFP_KERNEL);
+ if (!chan->seg_v) {
+ dev_err(chan->dev,
+ "unable to allocate channel %d descriptors\n",
+ chan->id);
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < XILINX_DMA_NUM_DESCS; i++) {
+ chan->seg_v[i].hw.next_desc =
+ lower_32_bits(chan->seg_p + sizeof(*chan->seg_v) *
+ ((i + 1) % XILINX_DMA_NUM_DESCS));
+ chan->seg_v[i].hw.next_desc_msb =
+ upper_32_bits(chan->seg_p + sizeof(*chan->seg_v) *
+ ((i + 1) % XILINX_DMA_NUM_DESCS));
+ chan->seg_v[i].phys = chan->seg_p +
+ sizeof(*chan->seg_v) * i;
+ list_add_tail(&chan->seg_v[i].node,
+ &chan->free_seg_list);
+ }
} else if (chan->xdev->dma_config->dmatype == XDMA_TYPE_CDMA) {
chan->desc_pool = dma_pool_create("xilinx_cdma_desc_pool",
chan->dev,
@@ -834,7 +887,8 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
0);
}
- if (!chan->desc_pool) {
+ if (!chan->desc_pool &&
+ (chan->xdev->dma_config->dmatype != XDMA_TYPE_AXIDMA)) {
dev_err(chan->dev,
"unable to allocate channel %d descriptor pool\n",
chan->id);
@@ -843,22 +897,20 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
/*
- * For AXI DMA case after submitting a pending_list, keep
- * an extra segment allocated so that the "next descriptor"
- * pointer on the tail descriptor always points to a
- * valid descriptor, even when paused after reaching taildesc.
- * This way, it is possible to issue additional
- * transfers without halting and restarting the channel.
- */
- chan->seg_v = xilinx_axidma_alloc_tx_segment(chan);
-
- /*
* For cyclic DMA mode we need to program the tail Descriptor
* register with a value which is not a part of the BD chain
* so allocating a desc segment during channel allocation for
* programming tail descriptor.
*/
- chan->cyclic_seg_v = xilinx_axidma_alloc_tx_segment(chan);
+ chan->cyclic_seg_v = dma_zalloc_coherent(chan->dev,
+ sizeof(*chan->cyclic_seg_v),
+ &chan->cyclic_seg_p, GFP_KERNEL);
+ if (!chan->cyclic_seg_v) {
+ dev_err(chan->dev,
+ "unable to allocate desc segment for cyclic DMA\n");
+ return -ENOMEM;
+ }
+ chan->cyclic_seg_v->phys = chan->cyclic_seg_p;
}
dma_cookie_init(dchan);
@@ -1198,7 +1250,7 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
{
struct xilinx_dma_tx_descriptor *head_desc, *tail_desc;
- struct xilinx_axidma_tx_segment *tail_segment, *old_head, *new_head;
+ struct xilinx_axidma_tx_segment *tail_segment;
u32 reg;
if (chan->err)
@@ -1217,21 +1269,6 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
tail_segment = list_last_entry(&tail_desc->segments,
struct xilinx_axidma_tx_segment, node);
- if (chan->has_sg && !chan->xdev->mcdma) {
- old_head = list_first_entry(&head_desc->segments,
- struct xilinx_axidma_tx_segment, node);
- new_head = chan->seg_v;
- /* Copy Buffer Descriptor fields. */
- new_head->hw = old_head->hw;
-
- /* Swap and save new reserve */
- list_replace_init(&old_head->node, &new_head->node);
- chan->seg_v = old_head;
-
- tail_segment->hw.next_desc = chan->seg_v->phys;
- head_desc->async_tx.phys = new_head->phys;
- }
-
reg = dma_ctrl_read(chan, XILINX_DMA_REG_DMACR);
if (chan->desc_pendingcount <= XILINX_DMA_COALESCE_MAX) {
@@ -1729,7 +1766,7 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
{
struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
struct xilinx_dma_tx_descriptor *desc;
- struct xilinx_axidma_tx_segment *segment = NULL, *prev = NULL;
+ struct xilinx_axidma_tx_segment *segment = NULL;
u32 *app_w = (u32 *)context;
struct scatterlist *sg;
size_t copy;
@@ -1780,10 +1817,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
XILINX_DMA_NUM_APP_WORDS);
}
- if (prev)
- prev->hw.next_desc = segment->phys;
-
- prev = segment;
sg_used += copy;
/*
@@ -1797,7 +1830,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
segment = list_first_entry(&desc->segments,
struct xilinx_axidma_tx_segment, node);
desc->async_tx.phys = segment->phys;
- prev->hw.next_desc = segment->phys;
/* For the last DMA_MEM_TO_DEV transfer, set EOP */
if (chan->direction == DMA_MEM_TO_DEV) {
@@ -2346,6 +2378,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
INIT_LIST_HEAD(&chan->pending_list);
INIT_LIST_HEAD(&chan->done_list);
INIT_LIST_HEAD(&chan->active_list);
+ INIT_LIST_HEAD(&chan->free_seg_list);
/* Retrieve the channel properties from the device tree */
has_dre = of_property_read_bool(node, "xlnx,include-dre");
--
2.1.2
^ permalink raw reply related
* [PATCH 05/62] watchdog: bcm2835_wdt: Convert to use device managed functions and other improvements
From: Eric Anholt @ 2017-01-14 6:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484091325-9199-6-git-send-email-linux@roeck-us.net>
Guenter Roeck <linux@roeck-us.net> writes:
> Use device managed functions to simplify error handling, reduce
> source code size, improve readability, and reduce the likelyhood of bugs.
> Other improvements as listed below.
>
> The conversion was done automatically with coccinelle using the
> following semantic patches. The semantic patches and the scripts used
> to generate this commit log are available at
> https://github.com/groeck/coccinelle-patches
>
> - Drop assignments to otherwise unused variables
> - Replace of_iomap() with platform_get_resource() followed by
> devm_ioremap_resource()
Every time I see this pattern I wish we had a
devm_ioremap_platform_resource().
> - Replace &pdev->dev with dev if 'struct device *dev' is a declared
> variable
> - Use devm_watchdog_register_driver() to register watchdog device
> - Replace shutdown function with call to watchdog_stop_on_reboot()
I'm trusting you here that this last change is right. All the rest of
it looks good:
Acked-by: Eric Anholt <eric@anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170113/817a1030/attachment.sig>
^ 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