Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] net: thunderx: Fix issue with IRQ namimg
From: sunil.kovvuri at gmail.com @ 2016-09-22  9:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474535121-13958-1-git-send-email-sunil.kovvuri@gmail.com>

From: Sunil Goutham <sgoutham@cavium.com>

This patch fixes a regression caused by previous commit
'Use netdev's name for naming VF's interrupts' where
irq name exceeds 20 byte array if interface's name size
is large.

Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
---
 drivers/net/ethernet/cavium/thunder/nic.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 18d12d3..3042610 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -305,7 +305,7 @@ struct nicvf {
 	bool			msix_enabled;
 	u8			num_vec;
 	struct msix_entry	msix_entries[NIC_VF_MSIX_VECTORS];
-	char			irq_name[NIC_VF_MSIX_VECTORS][20];
+	char			irq_name[NIC_VF_MSIX_VECTORS][IFNAMSIZ + 15];
 	bool			irq_allocated[NIC_VF_MSIX_VECTORS];
 	cpumask_var_t		affinity_mask[NIC_VF_MSIX_VECTORS];
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/2] net: thunderx: Support for byte queue limits
From: sunil.kovvuri at gmail.com @ 2016-09-22  9:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474535121-13958-1-git-send-email-sunil.kovvuri@gmail.com>

From: Sunil Goutham <sgoutham@cavium.com>

This patch adds support for byte queue limits

Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
---
 drivers/net/ethernet/cavium/thunder/nicvf_main.c   | 11 ++++++--
 drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 30 ++++++++++++++--------
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 7d00162..453e3a0 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -516,7 +516,8 @@ static int nicvf_init_resources(struct nicvf *nic)
 static void nicvf_snd_pkt_handler(struct net_device *netdev,
 				  struct cmp_queue *cq,
 				  struct cqe_send_t *cqe_tx,
-				  int cqe_type, int budget)
+				  int cqe_type, int budget,
+				  unsigned int *tx_pkts, unsigned int *tx_bytes)
 {
 	struct sk_buff *skb = NULL;
 	struct nicvf *nic = netdev_priv(netdev);
@@ -547,6 +548,8 @@ static void nicvf_snd_pkt_handler(struct net_device *netdev,
 		}
 		nicvf_put_sq_desc(sq, hdr->subdesc_cnt + 1);
 		prefetch(skb);
+		(*tx_pkts)++;
+		*tx_bytes += skb->len;
 		napi_consume_skb(skb, budget);
 		sq->skbuff[cqe_tx->sqe_ptr] = (u64)NULL;
 	} else {
@@ -662,6 +665,7 @@ static int nicvf_cq_intr_handler(struct net_device *netdev, u8 cq_idx,
 	struct cmp_queue *cq = &qs->cq[cq_idx];
 	struct cqe_rx_t *cq_desc;
 	struct netdev_queue *txq;
+	unsigned int tx_pkts = 0, tx_bytes = 0;
 
 	spin_lock_bh(&cq->lock);
 loop:
@@ -701,7 +705,7 @@ loop:
 		case CQE_TYPE_SEND:
 			nicvf_snd_pkt_handler(netdev, cq,
 					      (void *)cq_desc, CQE_TYPE_SEND,
-					      budget);
+					      budget, &tx_pkts, &tx_bytes);
 			tx_done++;
 		break;
 		case CQE_TYPE_INVALID:
@@ -730,6 +734,9 @@ done:
 		netdev = nic->pnicvf->netdev;
 		txq = netdev_get_tx_queue(netdev,
 					  nicvf_netdev_qidx(nic, cq_idx));
+		if (tx_pkts)
+			netdev_tx_completed_queue(txq, tx_pkts, tx_bytes);
+
 		nic = nic->pnicvf;
 		if (netif_tx_queue_stopped(txq) && netif_carrier_ok(netdev)) {
 			netif_tx_start_queue(txq);
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
index 178c5c7..a4fc501 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
@@ -1082,6 +1082,24 @@ static inline void nicvf_sq_add_cqe_subdesc(struct snd_queue *sq, int qentry,
 	imm->len = 1;
 }
 
+static inline void nicvf_sq_doorbell(struct nicvf *nic, struct sk_buff *skb,
+				     int sq_num, int desc_cnt)
+{
+	struct netdev_queue *txq;
+
+	txq = netdev_get_tx_queue(nic->pnicvf->netdev,
+				  skb_get_queue_mapping(skb));
+
+	netdev_tx_sent_queue(txq, skb->len);
+
+	/* make sure all memory stores are done before ringing doorbell */
+	smp_wmb();
+
+	/* Inform HW to xmit all TSO segments */
+	nicvf_queue_reg_write(nic, NIC_QSET_SQ_0_7_DOOR,
+			      sq_num, desc_cnt);
+}
+
 /* Segment a TSO packet into 'gso_size' segments and append
  * them to SQ for transfer
  */
@@ -1141,12 +1159,8 @@ static int nicvf_sq_append_tso(struct nicvf *nic, struct snd_queue *sq,
 	/* Save SKB in the last segment for freeing */
 	sq->skbuff[hdr_qentry] = (u64)skb;
 
-	/* make sure all memory stores are done before ringing doorbell */
-	smp_wmb();
+	nicvf_sq_doorbell(nic, skb, sq_num, desc_cnt);
 
-	/* Inform HW to xmit all TSO segments */
-	nicvf_queue_reg_write(nic, NIC_QSET_SQ_0_7_DOOR,
-			      sq_num, desc_cnt);
 	nic->drv_stats.tx_tso++;
 	return 1;
 }
@@ -1219,12 +1233,8 @@ doorbell:
 		nicvf_sq_add_cqe_subdesc(sq, qentry, tso_sqe, skb);
 	}
 
-	/* make sure all memory stores are done before ringing doorbell */
-	smp_wmb();
+	nicvf_sq_doorbell(nic, skb, sq_num, subdesc_cnt);
 
-	/* Inform HW to xmit new packet */
-	nicvf_queue_reg_write(nic, NIC_QSET_SQ_0_7_DOOR,
-			      sq_num, subdesc_cnt);
 	return 1;
 
 append_fail:
-- 
2.7.4

^ permalink raw reply related

* [PATCHv2] arm64: Correctly bounds check virt_addr_valid
From: Will Deacon @ 2016-09-22  9:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474496704-30541-1-git-send-email-labbott@redhat.com>

On Wed, Sep 21, 2016 at 03:25:04PM -0700, Laura Abbott wrote:
> 
> virt_addr_valid is supposed to return true if and only if virt_to_page
> returns a valid page structure. The current macro does math on whatever
> address is given and passes that to pfn_valid to verify. vmalloc and
> module addresses can happen to generate a pfn that 'happens' to be
> valid. Fix this by only performing the pfn_valid check on addresses that
> have the potential to be valid.
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Laura Abbott <labbott@redhat.com>
> ---
> v2: Properly parenthesize macro arguments. Re-factor to common macro.
> 
> Also in case it wasn't clear, there's no need to try and squeeze this
> into 4.8. Hardened usercopy should have all the checks, this is just for
> full correctness.

Thanks, I'll push this onto for-next/core later today.

Will

^ permalink raw reply

* [PATCH] PCI: rockchip: fix uninitialized variable use
From: Arnd Bergmann @ 2016-09-22  9:39 UTC (permalink / raw)
  To: linux-arm-kernel

The newly added pcie-rockchip driver fails to initialize the
io_size variable if the DT doesn't provide ranges for the PCI
I/O space, as found by building it with -Wmaybe-uninitialized:

drivers/pci/host/pcie-rockchip.c: In function 'rockchip_pcie_probe':
drivers/pci/host/pcie-rockchip.c:1007:6: warning: 'io_size' may be used uninitialized in this function [-Wmaybe-uninitialized]

This adds an appropriate initialization immediately in front of
the loop, so the io_size is zero as expected afterwards for that
case.

Fixes: abe17181b16f ("PCI: rockchip: Add Rockchip PCIe controller support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/pci/host/pcie-rockchip.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index c3593e633ccd..8bedc1e1ef80 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -1078,6 +1078,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 		goto err_vpcie;
 
 	/* Get the I/O and memory ranges from DT */
+	io_size = 0;
 	resource_list_for_each_entry(win, &res) {
 		switch (resource_type(win->res)) {
 		case IORESOURCE_IO:
-- 
2.9.0

^ permalink raw reply related

* [RESEND PATCH v7 1/2] sdhci-of-arasan: Add device tree parameter xlnx,fails-without-test-cd bit
From: Ulf Hansson @ 2016-09-22  9:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474038102-20937-1-git-send-email-zach.brown@ni.com>

On 16 September 2016 at 17:01, Zach Brown <zach.brown@ni.com> wrote:
> The sdhci controller on xilinx zynq devices will not function unless
> the CD bit is provided. http://www.xilinx.com/support/answers/61064.html
> In cases where it is impossible to provide the CD bit in hardware,
> setting the controller to test mode and then setting inserted to true
> will get the controller to function without the CD bit.
>
> The device property "xlnx,fails-without-test-cd" will let the arasan
> driver know the controller does not have the CD line wired and that the
> controller does not function without it.
>
> Signed-off-by: Zach Brown <zach.brown@ni.com>

Thanks, applied for next! I took the liberty to shrink the commit msg
header bit and to add "dt: " prefix.

Kind regards
Uffe

> ---
> v2:
>  * improved commit messages
>  * removed fake-cd device property
>  * removed fake-cd quirk
>  * use broken-cd device property
>  * documented new usage of broken-cd
> v3:
>  * removed new usage of broken-cd
>  * created fails-without-test-cd device property
>  * created arasan controller specific quirk
> v4:
>  * changed fails-without-test-cd to xlnx-fails-without-test-cd
>  * removed extra blank line
> v5:
>  * Fixed style mistake
>  * Changed (1 << 0 ) to BIT(0)
> v6:
>  * Fixed style mistakes
>  * Condensed unnecessarily long variable names
>  * Removed line wraps that were no longer necessary.
>  * Rebased changes off Ulf's mmc tree's next branch.
> v7:
>  * Removed erroneous re-creation of Optional Properties section
>  * Changed xlnx-fails-without-test-cd to xlnx,fails-without-test-cd
>  * Changed of_get_property to of_property_read_bool
>
>
>  Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index 3404afa..49df630 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -36,6 +36,9 @@ Optional Properties:
>    - #clock-cells: If specified this should be the value <0>.  With this property
>      in place we will export a clock representing the Card Clock.  This clock
>      is expected to be consumed by our PHY.  You must also specify
> +  - xlnx,fails-without-test-cd: when present, the controller doesn't work when
> +    the CD line is not connected properly, and the line is not connected
> +    properly. Test mode can be used to force the controller to function.
>
>  Example:
>         sdhci at e0100000 {
> --
> 2.7.4
>

^ permalink raw reply

* [RESEND PATCH v7 2/2] sdhci-of-arasan: Set controller to test mode when xlnx,fails-without-test-cd is present
From: Ulf Hansson @ 2016-09-22  9:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474038102-20937-2-git-send-email-zach.brown@ni.com>

On 16 September 2016 at 17:01, Zach Brown <zach.brown@ni.com> wrote:
> The sdhci controller on xilinx zynq devices will not function unless
> the CD bit is provided. http://www.xilinx.com/support/answers/61064.html
> In cases where it is impossible to provide the CD bit in hardware,
> setting the controller to test mode and then setting inserted to true
> will get the controller to function without the CD bit.
>
> When the device has the property xlnx,fails-without-test-cd the driver
> changes the controller to test mode and sets test inserted to true to
> make the controller function.
>
> Signed-off-by: Zach Brown <zach.brown@ni.com>

Thanks, applied for next! I took the liberty to shrink the commit msg
header bit and to add "mmc: " prefix.

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-of-arasan.c | 27 ++++++++++++++++++++++++++-
>  drivers/mmc/host/sdhci.h           |  2 ++
>  2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 33601a8..da8e40a 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -26,6 +26,7 @@
>  #include <linux/phy/phy.h>
>  #include <linux/regmap.h>
>  #include "sdhci-pltfm.h"
> +#include <linux/of.h>
>
>  #define SDHCI_ARASAN_CLK_CTRL_OFFSET   0x2c
>  #define SDHCI_ARASAN_VENDOR_REGISTER   0x78
> @@ -98,6 +99,10 @@ struct sdhci_arasan_data {
>
>         struct regmap   *soc_ctl_base;
>         const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
> +       unsigned int    quirks; /* Arasan deviations from spec */
> +
> +/* Controller does not have CD wired and will not function normally without */
> +#define SDHCI_ARASAN_QUIRK_FORCE_CDTEST        BIT(0)
>  };
>
>  static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
> @@ -245,12 +250,27 @@ static void sdhci_arasan_hs400_enhanced_strobe(struct mmc_host *mmc,
>         writel(vendor, host->ioaddr + SDHCI_ARASAN_VENDOR_REGISTER);
>  }
>
> +void sdhci_arasan_reset(struct sdhci_host *host, u8 mask)
> +{
> +       u8 ctrl;
> +       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +       struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
> +
> +       sdhci_reset(host, mask);
> +
> +       if (sdhci_arasan->quirks & SDHCI_ARASAN_QUIRK_FORCE_CDTEST) {
> +               ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
> +               ctrl |= SDHCI_CTRL_CDTEST_INS | SDHCI_CTRL_CDTEST_EN;
> +               sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> +       }
> +}
> +
>  static struct sdhci_ops sdhci_arasan_ops = {
>         .set_clock = sdhci_arasan_set_clock,
>         .get_max_clock = sdhci_pltfm_clk_get_max_clock,
>         .get_timeout_clock = sdhci_arasan_get_timeout_clock,
>         .set_bus_width = sdhci_set_bus_width,
> -       .reset = sdhci_reset,
> +       .reset = sdhci_arasan_reset,
>         .set_uhs_signaling = sdhci_set_uhs_signaling,
>  };
>
> @@ -545,6 +565,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>         struct sdhci_host *host;
>         struct sdhci_pltfm_host *pltfm_host;
>         struct sdhci_arasan_data *sdhci_arasan;
> +       struct device_node *np = pdev->dev.of_node;
>
>         host = sdhci_pltfm_init(pdev, &sdhci_arasan_pdata,
>                                 sizeof(*sdhci_arasan));
> @@ -599,6 +620,10 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>         }
>
>         sdhci_get_of_property(pdev);
> +
> +       if (of_property_read_bool(np, "xlnx,fails-without-test-cd"))
> +               sdhci_arasan->quirks |= SDHCI_ARASAN_QUIRK_FORCE_CDTEST;
> +
>         pltfm_host->clk = clk_xin;
>
>         if (of_device_is_compatible(pdev->dev.of_node,
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index a2bc9e1..c722cd2 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -84,6 +84,8 @@
>  #define   SDHCI_CTRL_ADMA32    0x10
>  #define   SDHCI_CTRL_ADMA64    0x18
>  #define   SDHCI_CTRL_8BITBUS   0x20
> +#define  SDHCI_CTRL_CDTEST_INS 0x40
> +#define  SDHCI_CTRL_CDTEST_EN  0x80
>
>  #define SDHCI_POWER_CONTROL    0x29
>  #define  SDHCI_POWER_ON                0x01
> --
> 2.7.4
>

^ permalink raw reply

* [PATCH] tty/serial: atmel: fix fractional baud rate computation
From: Boris Brezillon @ 2016-09-22  9:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160922093904.5b738ad6@bbrezillon>

On Thu, 22 Sep 2016 09:39:04 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Thu, 22 Sep 2016 09:07:46 +0200
> Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> wrote:
> 
> > On Wed, Sep 21, 2016 at 12:44:14PM +0200, Nicolas Ferre wrote:  
> > > From: Alexey Starikovskiy <aystarik@gmail.com>
> > > 
> > > The problem with previous code was it rounded values in wrong
> > > place and produced wrong baud rate in some cases.
> > > 
> > > Signed-off-by: Alexey Starikovskiy <aystarik@gmail.com>
> > > [nicolas.ferre at atmel.com: port to newer kernel and add commit log]
> > > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > > ---
> > >  drivers/tty/serial/atmel_serial.c | 10 ++++++----
> > >  include/linux/atmel_serial.h      |  1 +
> > >  2 files changed, 7 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> > > index 5f550d9feed9..fd8aa1f4ba78 100644
> > > --- a/drivers/tty/serial/atmel_serial.c
> > > +++ b/drivers/tty/serial/atmel_serial.c
> > > @@ -2170,13 +2170,15 @@ static void atmel_set_termios(struct uart_port *port, struct ktermios *termios,
> > >  	 * accurately. This feature is enabled only when using normal mode.
> > >  	 * baudrate = selected clock / (8 * (2 - OVER) * (CD + FP / 8))
> > >  	 * Currently, OVER is always set to 0 so we get
> > > -	 * baudrate = selected clock (16 * (CD + FP / 8))
> > > +	 * baudrate = selected clock / (16 * (CD + FP / 8))
> > > +	 * then
> > > +	 * 8 CD + FP = selected clock / (2 * baudrate)
> > >  	 */
> > >  	if (atmel_port->has_frac_baudrate &&
> > >  	    (mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_NORMAL) {
> > > -		div = DIV_ROUND_CLOSEST(port->uartclk, baud);
> > > -		cd = div / 16;
> > > -		fp = DIV_ROUND_CLOSEST(div % 16, 2);
> > > +		div = DIV_ROUND_CLOSEST(port->uartclk, baud * 2);
> > > +		cd = div >> 3;
> > > +		fp = div & ATMEL_US_FP_MASK;    
> > 
> > given baud = 115200 and uartclk = 5414300 this results in:
> > 
> > 	div = DIV_ROUND_CLOSEST(5414300, 115200 * 2) = 23
> > 	cd = 2
> > 	fp = 7  
> 
> How about:
> 
> 	div = DIV_ROUND_CLOSEST(port->uartclk, baud);
> 	cd = div / 16;
> 	fp = (div % 16) / 2;
> 
> 	best_baud = port->uartclk / ((16 * cd) +  (8 * fp));
> 
> 	/* Check if we can get a better approximation by rounding up. */
> 	if (div % 2) {
> 		int alt_baud, alt_fp, alt_cd;
> 
> 		alt_fp = fp++;
> 		alt_cd = cd;
> 		if (alt_fp > 7) {
> 			alt_cd++;
> 			alt_fp = 0;
> 		}
> 
> 		alt_baud = port->uartclk / ((16 * alt_cd) +  (8 *alt_fp));
> 		if (abs(best_baud - baud) > abs(alt_baud - baud)) {

After a lengthy discussion that happened on IRC (#armlinux), Uwe
proved me wrong. This should actually be


		/*
		 * Calculate the Error in the time domain:
		 * Error = (RealBaudPeriod - ExpectedBaudPeriod) /
		 *	   ExpectedBaudPeriod;
		 *
		 * which after conversion to the frequency domain gives:
		 * Error = 1 - (ExpectedBaudRate/RealBaudRate);
		 *
		 * and since we want to compare 2 errors and avoid
		 * approximation, we have:
		 *
		 * if (RealBaudRate2 * (RealBaudRate1 - ExpectedBaudRate) <
		 *     RealBaudRate1 * (RealBaudRate2 - ExpectedBaudRate))
		 *	...
		 * 
		 */
		if (alt_baud * abs(best_baud - baud) >
		    best_baud * abs(alt_baud - baud))

Thanks for your patience ;-).

> 			best_baud = alt_baud;
> 			fp = alt_fp;
> 			cd = alt_cd;
> 		}
> 	}
> 
> > 
> > which yields a rate of 5414300 / 46 = 117702.17. With cd = 3 and fp = 0
> > however the resulting rate is 5414300 / 48 = 112797.92.
> > 
> > Which one is better?
> >   
> > >  	} else {
> > >  		cd = uart_get_divisor(port, baud);
> > >  	}
> > > diff --git a/include/linux/atmel_serial.h b/include/linux/atmel_serial.h
> > > index f8e452aa48d7..bd2560502f3c 100644
> > > --- a/include/linux/atmel_serial.h
> > > +++ b/include/linux/atmel_serial.h
> > > @@ -119,6 +119,7 @@
> > >  #define ATMEL_US_BRGR		0x20	/* Baud Rate Generator Register */
> > >  #define	ATMEL_US_CD		GENMASK(15, 0)	/* Clock Divider */
> > >  #define ATMEL_US_FP_OFFSET	16	/* Fractional Part */
> > > +#define ATMEL_US_FP_MASK	0x7    
> > 
> > Is there another user of this header? If not, this can be folded into
> > the driver.
> > 
> > Best regards
> > Uwe
> >   
> 

^ permalink raw reply

* [PATCH v2 2/3] devicetree: bindings: uart: Add new compatible string for ZynqMP
From: Greg KH @ 2016-09-22  9:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473930931-1034-2-git-send-email-navam@xilinx.com>

On Thu, Sep 15, 2016 at 02:45:30PM +0530, Nava kishore Manne wrote:
> From: Nava kishore Manne <nava.manne@xilinx.com>
> 
> Signed-off-by: Nava kishore Manne <navam@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> [stelford at cadence.com: cherry picked from
> https://github.com/Xilinx/linux-xlnx commit
> 37b8a9780766422b2437f5166ddef3767bb60887]
> Signed-off-by: Scott Telford <stelford@cadence.com>

I can't take patches without any changelog comment at all, sorry.

greg k-h

^ permalink raw reply

* [PATCHv2 2/3] tty/serial: at91: fix hardware handshake with GPIOs
From: Greg Kroah-Hartman @ 2016-09-22  9:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160912094733.21501-3-richard.genoud@gmail.com>

On Mon, Sep 12, 2016 at 11:47:32AM +0200, Richard Genoud wrote:
> Commit 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when
> hardware handshake is enabled") broke the hardware handshake when GPIOs
> where used.
> 
> Hardware handshake with GPIOs used to work before this commit because
> the CRTSCTS flag (termios->c_cflag) was set, but not the
> ATMEL_US_USMODE_HWHS flag (controller register) ; so hardware handshake
> enabled, but not handled by the controller.
> 
> This commit restores this behaviour.
> 
> NB: -stable is not Cced because it doesn't cleanly apply on 4.1+
> and it will also need previous commit:
> "serial: mctrl_gpio: implement mctrl_gpio_use_rtscts"
> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Fixes: 1cf6e8fc8341 ("tty/serial: at91: fix RTS line management when hardware handshake is enabled")
> ---
>  drivers/tty/serial/atmel_serial.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)

This patch doesn't apply to my tree, are you sure it's up to date?

Can you please refresh this whole series against linux-next and resend?

thanks,

greg k-h

^ permalink raw reply

* [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-specific register range for ACPI case
From: Lorenzo Pieralisi @ 2016-09-22  9:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160921180457.GB20006@localhost>

On Wed, Sep 21, 2016 at 01:04:57PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 21, 2016 at 03:05:49PM +0100, Lorenzo Pieralisi wrote:
> > On Tue, Sep 20, 2016 at 02:17:44PM -0500, Bjorn Helgaas wrote:
> > > On Tue, Sep 20, 2016 at 04:09:25PM +0100, Ard Biesheuvel wrote:
> > 
> > [...]
> > 
> > > > None of these platforms can be fixed entirely in software, and given
> > > > that we will not be adding quirks for new broken hardware, we should
> > > > ask ourselves whether having two versions of a quirk, i.e., one for
> > > > broken hardware + currently shipping firmware, and one for the same
> > > > broken hardware with fixed firmware is really an improvement over what
> > > > has been proposed here.
> > > 
> > > We're talking about two completely different types of quirks:
> > > 
> > >   1) MCFG quirks to use memory-mapped config space that doesn't quite
> > >      conform to the ECAM model in the PCIe spec, and
> > > 
> > >   2) Some yet-to-be-determined method to describe address space
> > >      consumed by a bridge.
> > > 
> > > The first two patches of this series are a nice implementation for 1).
> > > The third patch (ThunderX-specific) is one possibility for 2), but I
> > > don't like it because there's no way for generic software like the
> > > ACPI core to discover these resources.
> > 
> > Ok, so basically this means that to implement (2) we need to assign
> > some sort of _HID to these quirky PCI bridges (so that we know what
> > device they represent and we can retrieve their _CRS). I take from
> > this discussion that the goal is to make sure that all non-config
> > resources have to be declared through _CRS device objects, which is
> > fine but that requires a FW update (unless we can fabricate ACPI
> > devices and corresponding _CRS in the kernel whenever we match a
> > given MCFG table signature).
> 
> All resources consumed by ACPI devices should be declared through
> _CRS.  If you want to fabricate ACPI devices or _CRS via kernel
> quirks, that's fine with me.  This could be triggered via MCFG
> signature, DMI info, host bridge _HID, etc.

I think the PNP quirk approach + PNP0c02 resource put forward by Gab
is enough.

> > We discussed this already and I think we should make a decision:
> > 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/414722.html
> > 
> > > > > I'd like to step back and come up with some understanding of how
> > > > > non-broken firmware *should* deal with this issue.  Then, if we *do*
> > > > > work around this particular broken firmware in the kernel, it would be
> > > > > nice to do it in a way that fits in with that understanding.
> > > > >
> > > > > For example, if a companion ACPI device is the preferred solution, an
> > > > > ACPI quirk could fabricate a device with the required resources.  That
> > > > > would address the problem closer to the source and make it more likely
> > > > > that the rest of the system will work correctly: /proc/iomem could
> > > > > make sense, things that look at _CRS generically would work (e.g,
> > > > > /sys/, an admittedly hypothetical "lsacpi", etc.)
> > > > >
> > > > > Hard-coding stuff in drivers is a point solution that doesn't provide
> > > > > any guidance for future platforms and makes it likely that the hack
> > > > > will get copied into even more drivers.
> > > > >
> > > > 
> > > > OK, I see. But the guidance for future platforms should be 'do not
> > > > rely on quirks', and what I am arguing here is that the more we polish
> > > > up this code and make it clean and reusable, the more likely it is
> > > > that will end up getting abused by new broken hardware that we set out
> > > > to reject entirely in the first place.
> > > > 
> > > > So of course, if the quirk involves claiming resources, let's make
> > > > sure that this occurs in the cleanest and most compliant way possible.
> > > > But any factoring/reuse concerns other than for the current crop of
> > > > broken hardware should be avoided imo.
> > > 
> > > If future hardware is completely ECAM-compliant and we don't need any
> > > more MCFG quirks, that would be great.
> > 
> > Yes.
> > 
> > > But we'll still need to describe that memory-mapped config space
> > > somewhere.  If that's done with PNP0C02 or similar devices (as is done
> > > on my x86 laptop), we'd be all set.
> > 
> > I am not sure I understand what you mean here. Are you referring
> > to MCFG regions reported as PNP0c02 resources through its _CRS ?
> 
> Yes.  PCI Firmware Spec r3.0, Table 4-2, note 2 says address ranges
> reported via MCFG or _CBA should be reserved by _CRS of a PNP0C02
> device.

Ok, that's agreed. It goes without saying that since you are quoting
the PCI spec, if FW fails to report MCFG regions in a PNP0c02 device
_CRS I will consider that a FW bug.

> > IIUC PNP0C02 is a reservation mechanism, but it does not help us
> > associate its _CRS to a specific PCI host bridge instance, right ?
> 
> Gab proposed a hierarchy that *would* associate a PNP0C02 device with
> a PCI bridge:
> 
>   Device (PCI1) {
>     Name (_HID, "HISI0080") // PCI Express Root Bridge
>     Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
>     Method (_CRS, 0, Serialized) { // Root complex resources (windows) }
>     Device (RES0) {
>       Name (_HID, "HISI0081") // HiSi PCIe RC config base address
>       Name (_CID, "PNP0C02")  // Motherboard reserved resource
>       Name (_CRS, ResourceTemplate () { ... }
>     }
>   }
> 
> That's a possibility.  The PCI Firmware Spec suggests putting RES0 at
> the root (under \_SB), but I don't know why.
> 
> Putting it at the root means we couldn't generically associate it with
> a bridge, although I could imagine something like this:
> 
>   Device (RES1) {
>     Name (_HID, "HISI0081") // HiSi PCIe RC config base address
>     Name (_CID, "PNP0C02")  // Motherboard reserved resource
>     Name (_CRS, ResourceTemplate () { ... }
>     Method (BRDG) { "PCI1" }  // hand-wavy ASL
>   }
>   Device (PCI1) {
>     Name (_HID, "HISI0080") // PCI Express Root Bridge
>     Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
>     Method (_CRS, 0, Serialized) { // Root complex resources (windows) }
>   }
> 
> Where you could search PNP0C02 devices for a cookie that matched the
> host bridge.o

Ok, I am fine with both and I think we are converging, but the way
to solve this problem has to be uniform for all ARM partners (and
not only ARM). Two points here:

1) Adding a device/subdevice allows people to add a _CRS reporting the
   non-window bridge resources. Fine. It also allows people to chuck in
   there all sorts of _DSD properties to describe their PCI host bridge
   as it is done with DT properties (those _DSD can contain eg clocks
   etc.), this may be tempting (so that they can reuse the same DT
   driver and do not have to update their firmware) but I want to be
   clear here: that must not happen. So, a subdevice with a _CRS to
   report resources, yes, but it will stop there.
2) It is unclear to me how to formalize the above. People should not
   write FW by reading the PCI mailing list, so these guidelines have to
   be written, somehow. I do not want to standardize quirks, I want
   to prevent random ACPI table content, which is different.
   Should I report this to the ACPI spec working group ? If we do
   not do that everyone will go solve this problem as they deem fit.

[...]

> > For FW that is immutable I really do not see what we can do apart
> > from hardcoding the non-config resources (consumed by a bridge),
> > somehow.
> 
> Right.  Well, I assume you mean we should hard-code "non-window
> resources consumed directly by a bridge".  If firmware in the field is
> broken, we should work around it, and that may mean hard-coding some
> resources.
> 
> My point is that the hard-coding should not be buried in a driver
> where it's invisible to the rest of the kernel.  If we hard-code it in
> a quirk that adds _CRS entries, then the kernel will work just like it
> would if the firmware had been correct in the first place.  The
> resource will appear in /sys/devices/pnp*/*/resources and /proc/iomem,
> and if we ever used _SRS to assign or move ACPI devices, we would know
> to avoid the bridge resource.

We are in complete agreement here.

Thanks,
Lorenzo

^ permalink raw reply

* [PATCH v4 09/10] ARM: gic-v3: Introduce 32-to-64-bit mappings for GICv3 cpu registers
From: Marc Zyngier @ 2016-09-22  9:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473691764-29424-10-git-send-email-vladimir.murzin@arm.com>

On 12/09/16 15:49, Vladimir Murzin wrote:
> vgic-v3 save/restore routines are written in such way that they map
> arm64 system register naming nicely, but it does not fit to arm
> world. To keep virt/kvm/arm/hyp/vgic-v3-sr.c untouched we create a
> mapping with a function for each register mapping the 32-bit to the
> 64-bit accessors.
> 
> Please, note that 64-bit wide ICH_LR is split in two 32-bit halves
> (ICH_LR and ICH_LRC) accessed independently.
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  arch/arm/include/asm/arch_gicv3.h |   64 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v4 10/10] ARM: KVM: Support vgic-v3
From: Marc Zyngier @ 2016-09-22  9:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473691764-29424-11-git-send-email-vladimir.murzin@arm.com>

On 12/09/16 15:49, Vladimir Murzin wrote:
> This patch allows to build and use vgic-v3 in 32-bit mode.
> 
> Unfortunately, it can not be split in several steps without extra
> stubs to keep patches independent and bisectable.  For instance,
> virt/kvm/arm/vgic/vgic-v3.c uses function from vgic-v3-sr.c, handling
> access to GICv3 cpu interface from the guest requires vgic_v3.vgic_sre
> to be already defined.
> 
> It is how support has been done:
> 
> * handle SGI requests from the guest
> 
> * report configured SRE on access to GICv3 cpu interface from the guest
> 
> * required vgic-v3 macros are provided via uapi.h
> 
> * static keys are used to select GIC backend
> 
> * to make vgic-v3 build KVM_ARM_VGIC_V3 guard is removed along with
>   the static inlines
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  arch/arm/include/asm/kvm_asm.h      |    3 ++
>  arch/arm/include/asm/kvm_host.h     |    5 ++++
>  arch/arm/include/asm/kvm_hyp.h      |    3 ++
>  arch/arm/include/uapi/asm/kvm.h     |    7 +++++
>  arch/arm/kvm/Makefile               |    2 ++
>  arch/arm/kvm/coproc.c               |   35 +++++++++++++++++++++++
>  arch/arm/kvm/hyp/Makefile           |    1 +
>  arch/arm/kvm/hyp/switch.c           |   12 ++++++--
>  arch/arm64/kvm/Kconfig              |    4 ---
>  include/kvm/arm_vgic.h              |    8 ------
>  virt/kvm/arm/vgic/vgic-kvm-device.c |    8 ------
>  virt/kvm/arm/vgic/vgic-mmio.c       |    2 --
>  virt/kvm/arm/vgic/vgic-mmio.h       |    2 --
>  virt/kvm/arm/vgic/vgic.h            |   54 -----------------------------------
>  14 files changed, 66 insertions(+), 80 deletions(-)

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v4 08/10] ARM: Move system register accessors to asm/cp15.h
From: Marc Zyngier @ 2016-09-22  9:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473691764-29424-9-git-send-email-vladimir.murzin@arm.com>

On 12/09/16 15:49, Vladimir Murzin wrote:
> Headers linux/irqchip/arm-gic.v3.h and arch/arm/include/asm/kvm_hyp.h
> are included in virt/kvm/arm/hyp/vgic-v3-sr.c and both define macros
> called __ACCESS_CP15 and __ACCESS_CP15_64 which obviously creates a
> conflict. These macros were introduced independently for GIC and KVM
> and, in fact, do the same thing.
> 
> As an option we could add prefixes to KVM and GIC version of macros so
> they won't clash, but it'd introduce code duplication.  Alternatively,
> we could keep macro in, say, GIC header and include it in KVM one (or
> vice versa), but such dependency would not look nicer.
> 
> So we follow arm64 way (it handles this via sysreg.h) and move only
> single set of macros to asm/cp15.h
> 
> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  arch/arm/include/asm/arch_gicv3.h |   27 +++++++++++----------------
>  arch/arm/include/asm/cp15.h       |   15 +++++++++++++++
>  arch/arm/include/asm/kvm_hyp.h    |   15 +--------------
>  3 files changed, 27 insertions(+), 30 deletions(-)

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v4 07/10] ARM: Introduce MPIDR_LEVEL_SHIFT macro
From: Marc Zyngier @ 2016-09-22  9:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473691764-29424-8-git-send-email-vladimir.murzin@arm.com>

On 12/09/16 15:49, Vladimir Murzin wrote:
> vgic-v3 driver uses architecture specific MPIDR_LEVEL_SHIFT macro to
> encode the affinity in a form compatible with ICC_SGI* registers.
> Unfortunately, that macro is missing on ARM, so let's add it.
> 
> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  arch/arm/include/asm/cputype.h |    1 +
>  1 file changed, 1 insertion(+)

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v4 06/10] KVM: arm: vgic: Support 64-bit data manipulation on 32-bit host systems
From: Marc Zyngier @ 2016-09-22 10:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473691764-29424-7-git-send-email-vladimir.murzin@arm.com>

On 12/09/16 15:49, Vladimir Murzin wrote:
> We have couple of 64-bit registers defined in GICv3 architecture, so
> unsigned long accesses to these registers will only access a single
> 32-bit part of that regitser. On the other hand these registers can't
> be accessed as 64-bit with a single instruction like ldrd/strd or
> ldmia/stmia if we run a 32-bit host because KVM does not support
> access to MMIO space done by these instructions.
> 
> It means that a 32-bit guest accesses these registers in 32-bit
> chunks, so the only thing we need to do is to ensure that
> extract_bytes() always takes 64-bit data.
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  virt/kvm/arm/vgic/vgic-mmio-v3.c |    2 +-
>  virt/kvm/arm/vgic/vgic-mmio.h    |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v4 01/10] arm64: KVM: Use static keys for selecting the GIC backend
From: Marc Zyngier @ 2016-09-22 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473691764-29424-2-git-send-email-vladimir.murzin@arm.com>

On 12/09/16 15:49, Vladimir Murzin wrote:
> Currently GIC backend is selected via alternative framework and this
> is fine. We are going to introduce vgic-v3 to 32-bit world and there
> we don't have patching framework in hand, so we can either check
> support for GICv3 every time we need to choose which backend to use or
> try to optimise it by using static keys. The later looks quite
> promising because we can share logic involved in selecting GIC backend
> between architectures if both uses static keys.
> 
> This patch moves arm64 from alternative to static keys framework for
> selecting GIC backend. For that we embed static key into vgic_global
> and enable the key during vgic initialisation based on what has
> already been exposed by the host GIC driver.
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  arch/arm64/kvm/hyp/switch.c   |   21 +++++++++++----------
>  include/kvm/arm_vgic.h        |    4 ++++
>  virt/kvm/arm/vgic/vgic-init.c |    4 ++++
>  virt/kvm/arm/vgic/vgic.c      |    2 +-
>  4 files changed, 20 insertions(+), 11 deletions(-)

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v4 05/10] KVM: arm: vgic: Fix compiler warnings when built for 32-bit
From: Marc Zyngier @ 2016-09-22 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473691764-29424-6-git-send-email-vladimir.murzin@arm.com>

On 12/09/16 15:49, Vladimir Murzin wrote:
> Well, this patch is looking ahead of time, but we'll get following
> compiler warnings as soon as we introduce vgic-v3 to 32-bit world
> 
>   CC      arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.o
> arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.c: In function 'vgic_mmio_read_v3r_typer':
> arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.c:184:35: warning: left shift count >= width of type [-Wshift-count-overflow]
>   value = (mpidr & GENMASK(23, 0)) << 32;
>                                    ^
> In file included from ./include/linux/kernel.h:10:0,
>                  from ./include/asm-generic/bug.h:13,
>                  from ./arch/arm/include/asm/bug.h:59,
>                  from ./include/linux/bug.h:4,
>                  from ./include/linux/io.h:23,
>                  from ./arch/arm/include/asm/arch_gicv3.h:23,
>                  from ./include/linux/irqchip/arm-gic-v3.h:411,
>                  from arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.c:14:
> arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.c: In function 'vgic_v3_dispatch_sgi':
> ./include/linux/bitops.h:6:24: warning: left shift count >= width of type [-Wshift-count-overflow]
>  #define BIT(nr)   (1UL << (nr))
>                         ^
> arch/arm/kvm/../../../virt/kvm/arm/vgic/vgic-mmio-v3.c:614:20: note: in expansion of macro 'BIT'
>   broadcast = reg & BIT(ICC_SGI1R_IRQ_ROUTING_MODE_BIT);
>                     ^
> Let's fix them now.
> 
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> ---
>  virt/kvm/arm/vgic/vgic-mmio-v3.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v26 1/7] arm64: kdump: reserve memory for crash dump kernel
From: Matthias Bruger @ 2016-09-22 10:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160907042908.6232-2-takahiro.akashi@linaro.org>



On 09/07/2016 06:29 AM, AKASHI Takahiro wrote:
> On the startup of primary kernel, the memory region used by crash dump
> kernel must be specified by "crashkernel=" kernel parameter.
> reserve_crashkernel() will allocate and reserve the region for later use.
>
> User space tools, like kexec-tools, will be able to find that region as
> 	- "Crash kernel" in /proc/iomem, or
> 	- "linux,crashkernel-base" and "linux,crashkernel-size" under
> 	  /sys/firmware/devicetree/base/chosen
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Signed-off-by: Mark Salter <msalter@redhat.com>
> Signed-off-by: Pratyush Anand <panand@redhat.com>
> Reviewed-by: James Morse <james.morse@arm.com>
> ---
>  arch/arm64/kernel/setup.c |   7 ++-
>  arch/arm64/mm/init.c      | 113 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 119 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 514b4e3..38589b5 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -31,7 +31,6 @@
>  #include <linux/screen_info.h>
>  #include <linux/init.h>
>  #include <linux/kexec.h>
> -#include <linux/crash_dump.h>
>  #include <linux/root_dev.h>
>  #include <linux/cpu.h>
>  #include <linux/interrupt.h>
> @@ -225,6 +224,12 @@ static void __init request_standard_resources(void)
>  		    kernel_data.end <= res->end)
>  			request_resource(res, &kernel_data);
>  	}
> +
> +#ifdef CONFIG_KEXEC_CORE
> +	/* User space tools will find "Crash kernel" region in /proc/iomem. */
> +	if (crashk_res.end)
> +		insert_resource(&iomem_resource, &crashk_res);
> +#endif
>  }
>
>  u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index bbb7ee7..dd273ec 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -29,11 +29,13 @@
>  #include <linux/gfp.h>
>  #include <linux/memblock.h>
>  #include <linux/sort.h>
> +#include <linux/of.h>
>  #include <linux/of_fdt.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dma-contiguous.h>
>  #include <linux/efi.h>
>  #include <linux/swiotlb.h>
> +#include <linux/kexec.h>
>
>  #include <asm/boot.h>
>  #include <asm/fixmap.h>
> @@ -76,6 +78,114 @@ static int __init early_initrd(char *p)
>  early_param("initrd", early_initrd);
>  #endif
>
> +#ifdef CONFIG_KEXEC_CORE
> +static unsigned long long crash_size, crash_base;
> +static struct property crash_base_prop = {
> +	.name = "linux,crashkernel-base",
> +	.length = sizeof(u64),
> +	.value = &crash_base
> +};
> +static struct property crash_size_prop = {
> +	.name = "linux,crashkernel-size",
> +	.length = sizeof(u64),
> +	.value = &crash_size,
> +};
> +
> +static int __init export_crashkernel(void)
> +{
> +	struct device_node *node;
> +	int ret;
> +
> +	if (!crashk_res.end)
> +		return 0;
> +
> +	crash_base = cpu_to_be64(crashk_res.start);
> +	crash_size = cpu_to_be64(crashk_res.end - crashk_res.start + 1);
> +

Shouldn't that be the same values as in reserve_crashkernel()?
IMHO this does not need to be recalculated here.

Regards,
Matthias

> +	/* Add /chosen/linux,crashkernel-* properties */
> +	node = of_find_node_by_path("/chosen");
> +	if (!node)
> +		return -ENOENT;
> +
> +	/*
> +	 * There might be existing crash kernel properties, but we can't
> +	 * be sure what's in them, so remove them.
> +	 */
> +	of_remove_property(node, of_find_property(node,
> +				"linux,crashkernel-base", NULL));
> +	of_remove_property(node, of_find_property(node,
> +				"linux,crashkernel-size", NULL));
> +
> +	ret = of_add_property(node, &crash_base_prop);
> +	if (ret)
> +		goto ret_err;
> +
> +	ret = of_add_property(node, &crash_size_prop);
> +	if (ret)
> +		goto ret_err;
> +
> +	return 0;
> +
> +ret_err:
> +	pr_warn("Exporting crashkernel region to device tree failed\n");
> +	return ret;
> +}
> +late_initcall(export_crashkernel);
> +
> +/*
> + * reserve_crashkernel() - reserves memory for crash kernel
> + *
> + * This function reserves memory area given in "crashkernel=" kernel command
> + * line parameter. The memory reserved is used by dump capture kernel when
> + * primary kernel is crashing.
> + */
> +static void __init reserve_crashkernel(void)
> +{
> +	int ret;
> +
> +	ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
> +				&crash_size, &crash_base);
> +	/* no crashkernel= or invalid value specified */
> +	if (ret || !crash_size)
> +		return;
> +
> +	if (crash_base == 0) {
> +		/* Current arm64 boot protocol requires 2MB alignment */
> +		crash_base = memblock_find_in_range(0, ARCH_LOW_ADDRESS_LIMIT,
> +				crash_size, SZ_2M);
> +		if (crash_base == 0) {
> +			pr_warn("Unable to allocate crashkernel (size:%llx)\n",
> +				crash_size);
> +			return;
> +		}
> +	} else {
> +		/* User specifies base address explicitly. */
> +		if (!memblock_is_region_memory(crash_base, crash_size) ||
> +			memblock_is_region_reserved(crash_base, crash_size)) {
> +			pr_warn("crashkernel has wrong address or size\n");
> +			return;
> +		}
> +
> +		if (!IS_ALIGNED(crash_base, SZ_2M)) {
> +			pr_warn("crashkernel base address is not 2MB aligned\n");
> +			return;
> +		}
> +	}
> +	memblock_reserve(crash_base, crash_size);
> +
> +	pr_info("Reserving %lldMB of memory at %lldMB for crashkernel\n",
> +		crash_size >> 20, crash_base >> 20);
> +
> +	crashk_res.start = crash_base;
> +	crashk_res.end = crash_base + crash_size - 1;
> +}
> +#else
> +static void __init reserve_crashkernel(void)
> +{
> +	;
> +}
> +#endif /* CONFIG_KEXEC_CORE */
> +
>  /*
>   * Return the maximum physical address for ZONE_DMA (DMA_BIT_MASK(32)). It
>   * currently assumes that for memory starting above 4G, 32-bit devices will
> @@ -296,6 +406,9 @@ void __init arm64_memblock_init(void)
>  		arm64_dma_phys_limit = max_zone_dma_phys();
>  	else
>  		arm64_dma_phys_limit = PHYS_MASK + 1;
> +
> +	reserve_crashkernel();
> +
>  	dma_contiguous_reserve(arm64_dma_phys_limit);
>
>  	memblock_allow_resize();
>

^ permalink raw reply

* [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
From: Mugunthan V N @ 2016-09-22 10:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1dbd7bbd-36ac-4433-79bf-8975a06dd07e@ti.com>

On Thursday 22 September 2016 11:48 AM, Vignesh R wrote:
> 
> 
> On Wednesday 21 September 2016 09:41 PM, Mugunthan V N wrote:
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>>  drivers/iio/adc/ti_am335x_adc.c      | 160 ++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/ti_am335x_tscadc.h |   7 ++
>>  2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>>  #include <linux/iio/buffer.h>
>>  #include <linux/iio/kfifo_buf.h>
>>  
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE		SZ_2K
>> +
>> +struct tiadc_dma {
>> +	/* Filter function */
>> +	dma_filter_fn		fn;
>> +	/* Parameter to the filter function */
>> +	void			*param;
> 
> These will not be needed with newer APIs (see below)
> 
>> +	struct dma_slave_config	conf;
>> +	struct dma_chan		*chan;
>> +	dma_addr_t		addr;
>> +	dma_cookie_t		cookie;
>> +	u8			*buf;
>> +	bool			valid_buf_seg;
>> +	int			buf_offset;
>> +	u8			fifo_thresh;
>> +};
>> +
>>  struct tiadc_device {
>>  	struct ti_tscadc_dev *mfd_tscadc;
>> +	struct tiadc_dma dma;
>>  	struct mutex fifo1_lock; /* to protect fifo access */
>>  	int channels;
>> +	int total_ch_enabled;
>>  	u8 channel_line[8];
>>  	u8 channel_step[8];
>>  	int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>>  	u16 *data = adc_dev->data;
>>  
>>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>>  	for (k = 0; k < fifo1count; k = k + i) {
>>  		for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>>  			read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>>  	return IRQ_HANDLED;
>>  }
>>  
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> +	struct iio_dev *indio_dev = param;
>> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>> +	u8 *data;
>> +	int i;
>> +
>> +	data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
>> +	dma->valid_buf_seg = !dma->valid_buf_seg;
>> +
>> +	for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> +		iio_push_to_buffers(indio_dev, data);
>> +		data += indio_dev->scan_bytes;
>> +	}
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>> +	struct dma_async_tx_descriptor *desc;
>> +
>> +	dma->valid_buf_seg = false;
>> +	dma->fifo_thresh = FIFO1_THRESHOLD;
>> +	/*
>> +	 * Make the fifo thresh as the multiple of total number of
>> +	 * channels enabled, so make sure that that cyclic DMA period
>> +	 * length is also a multiple of total number of channels
>> +	 * enabled. This ensures that no invalid data is reported
>> +	 * to the stack via iio_push_to_buffers().
>> +	 */
>> +	dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
> 
> Can we use rounddown(FIFO1_THRESHOLD + 1, adc_dev->total_ch_enabled)?
> 
>> +	dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> +	/* Make sure that period length is multiple of fifo thresh level */
>> +	dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> +					      sizeof(u16));
>> +
> 
> Can we use rounddown()?

Will change this in next version.

> 
>> +	dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> +	dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> +	desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> +					 dma->buf_offset * 2,
>> +					 dma->buf_offset, DMA_DEV_TO_MEM,
>> +					 DMA_PREP_INTERRUPT);
>> +	if (!desc)
>> +		return -EBUSY;
>> +
>> +	desc->callback = tiadc_dma_rx_complete;
>> +	desc->callback_param = indio_dev;
>> +
>> +	dma->cookie = dmaengine_submit(desc);
>> +
>> +	dma_async_issue_pending(dma->chan);
>> +
>> +	tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> +	tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> +	tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> +	return 0;
>> +}
>> +
>>  static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>>  static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	unsigned int enb = 0;
>>  	u8 bit;
>> +	u32 irq_enable;
>>  
>>  	tiadc_step_config(indio_dev);
>> -	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> +	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>>  		enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> +		adc_dev->total_ch_enabled++;
>> +	}
>>  	adc_dev->buffer_en_ch_steps = enb;
>>  
>> +	if (dma->chan)
>> +		tiadc_start_dma(indio_dev);
>> +
>>  	am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>  
>>  	tiadc_writel(adc_dev,  REG_IRQSTATUS, IRQENB_FIFO1THRES
>>  				| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> -	tiadc_writel(adc_dev,  REG_IRQENABLE, IRQENB_FIFO1THRES
>> -				| IRQENB_FIFO1OVRRUN);
>> +
>> +	irq_enable = IRQENB_FIFO1OVRRUN;
>> +	if (!dma->chan)
>> +		irq_enable |= IRQENB_FIFO1THRES;
>> +	tiadc_writel(adc_dev,  REG_IRQENABLE, irq_enable);
>>  
>>  	return 0;
>>  }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>>  static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	int fifo1count, i, read;
>>  
>>  	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>>  				IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>>  	am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>>  	adc_dev->buffer_en_ch_steps = 0;
>> +	adc_dev->total_ch_enabled = 0;
>> +	if (dma->chan) {
>> +		tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> +		dmaengine_terminate_async(dma->chan);
>> +	}
>>  
>>  	/* Flush FIFO of leftover data in the time it takes to disable adc */
>>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>>  	.driver_module = THIS_MODULE,
>>  };
>>  
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> +	return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> +			     struct tiadc_device *adc_dev)
>> +{
>> +	struct tiadc_dma	*dma = &adc_dev->dma;
>> +	dma_cap_mask_t		mask;
>> +
>> +	dma->fn = the_no_dma_filter_fn;
>> +
>> +	/* Default slave configuration parameters */
>> +	dma->conf.direction = DMA_DEV_TO_MEM;
>> +	dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> +	dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> +	dma_cap_zero(mask);
>> +	dma_cap_set(DMA_CYCLIC, mask);
>> +
>> +	/* Get a channel for RX */
>> +	dma->chan = dma_request_slave_channel_compat(mask,
>> +						     dma->fn, dma->param,
>> +						     adc_dev->mfd_tscadc->dev,
>> +						     "fifo1");
> 
> Please use dma_request_chan() API instead, this does not need
> dma_filter_fn and probe defer can be handled.

Okay

> 
>> +	if (!dma->chan)
>> +		return -ENODEV;
>> +
>> +	/* RX buffer */
>> +	dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> +				      &dma->addr, GFP_KERNEL);
>> +	if (!dma->buf)
>> +		goto err;
>> +
>> +	dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
> 
> Do we need _ratelimited? AFAICS, this print is called only once.

True, will change to dev_dbg.

> 
>> +
>> +	return 0;
>> +err:
>> +	dma_release_channel(dma->chan);
>> +
>> +	return -ENOMEM;
>> +}
>> +
>>  static int tiadc_parse_dt(struct platform_device *pdev,
>>  			  struct tiadc_device *adc_dev)
>>  {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>  
>>  	platform_set_drvdata(pdev, indio_dev);
>>  
>> +	err = tiadc_request_dma(pdev, adc_dev);
>> +	if (err && err != -ENODEV)
>> +		goto err_dma;
>> +
>>  	return 0;
>>  
>> +err_dma:
>> +	iio_device_unregister(indio_dev);
>>  err_buffer_unregister:
>>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>>  err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>>  {
>>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	u32 step_en;
>>  
>> +	if (dma->chan)
>> +		dma_release_channel(dma->chan);
> 
> dma_free_coherent() for dma->buf?

Oops missed, will fix in next version.

Regards
Mugunthan V N

> 
>>  	iio_device_unregister(indio_dev);
>>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>>  	tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>>  #define REG_IRQENABLE		0x02C
>>  #define REG_IRQCLR		0x030
>>  #define REG_IRQWAKEUP		0x034
>> +#define REG_DMAENABLE_SET	0x038
>> +#define REG_DMAENABLE_CLEAR	0x038
>>  #define REG_CTRL		0x040
>>  #define REG_ADCFSM		0x044
>>  #define REG_CLKDIV		0x04C
>> @@ -36,6 +38,7 @@
>>  #define REG_FIFO0THR		0xE8
>>  #define REG_FIFO1CNT		0xF0
>>  #define REG_FIFO1THR		0xF4
>> +#define REG_DMA1REQ		0xF8
>>  #define REG_FIFO0		0x100
>>  #define REG_FIFO1		0x200
>>  
>> @@ -126,6 +129,10 @@
>>  #define FIFOREAD_DATA_MASK (0xfff << 0)
>>  #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>  
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0		BIT(0)
>> +#define DMA_FIFO1		BIT(1)
>> +
>>  /* Sequencer Status */
>>  #define SEQ_STATUS BIT(5)
>>  #define CHARGE_STEP		0x11
>>
> 

^ permalink raw reply

* [PATCH] crypto: sun4i-ss: mark sun4i_hash() static
From: Herbert Xu @ 2016-09-22 10:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474203164-14884-1-git-send-email-baoyou.xie@linaro.org>

On Sun, Sep 18, 2016 at 08:52:44PM +0800, Baoyou Xie wrote:
> We get 1 warning when building kernel with W=1:
> drivers/crypto/sunxi-ss/sun4i-ss-hash.c:168:5: warning: no previous prototype for 'sun4i_hash' [-Wmissing-prototypes]
> 
> In fact, this function is only used in the file in which it is
> declared and don't need a declaration, but can be made static.
> So this patch marks it 'static'.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH 0/8] crypto: omap-sham: convert to sg based data engine
From: Herbert Xu @ 2016-09-22 10:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474298539-23897-1-git-send-email-t-kristo@ti.com>

On Mon, Sep 19, 2016 at 06:22:11PM +0300, Tero Kristo wrote:
> Hi,
> 
> This series converts the omap-sham buffer handling towards a scatterlist
> approach. This avoids the need to have a huge internal buffer within the
> driver, and also allows us to properly implement export/import for the
> driver. I tried splitting up the changes to some sane patches, but this
> was rather difficult due to the fact that this is largely a complete
> rewrite of portions of the driver. Patch #6 is a prime example of this
> being pretty large, but splitting this up would break bisectability.
> Hopefully the patch is still understandable though.
> 
> Crypto manager tests work fine at least on omap3/am3/am4/dra7 SoC:s.
> (Something is broken with test farm again and could not try omap2/omap4.)
> 
> Also tested tcrypt SHA performance on DRA7 and it seems to be working
> fine with different buffer sizes.
> 
> My test branch is also available here for interested parties:
> tree: https://github.com/t-kristo/linux-pm.git
> breanch: 4.8-rc1-crypto

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH] hwrng: omap - Only fail if pm_runtime_get_sync returns < 0
From: Herbert Xu @ 2016-09-22 10:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160920152540.3004-1-d-gerlach@ti.com>

On Tue, Sep 20, 2016 at 10:25:40AM -0500, Dave Gerlach wrote:
> Currently omap-rng checks the return value of pm_runtime_get_sync and
> reports failure if anything is returned, however it should be checking
> if ret < 0 as pm_runtime_get_sync return 0 on success but also can return
> 1 if the device was already active which is not a failure case. Only
> values < 0 are actual failures.
> 
> Fixes: 61dc0a446e5d ("hwrng: omap - Fix assumption that runtime_get_sync will always succeed")
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
From: Mugunthan V N @ 2016-09-22 10:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <94b46406-577e-9cec-8e79-1134581932c5@ti.com>

On Thursday 22 September 2016 12:50 PM, Peter Ujfalusi wrote:
> On 09/21/16 19:11, Mugunthan V N wrote:
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>>  drivers/iio/adc/ti_am335x_adc.c      | 160 ++++++++++++++++++++++++++++++++++-
>>  include/linux/mfd/ti_am335x_tscadc.h |   7 ++
>>  2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>>  #include <linux/iio/buffer.h>
>>  #include <linux/iio/kfifo_buf.h>
>>  
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE		SZ_2K
>> +
>> +struct tiadc_dma {
>> +	/* Filter function */
>> +	dma_filter_fn		fn;
>> +	/* Parameter to the filter function */
>> +	void			*param;
>> +	struct dma_slave_config	conf;
>> +	struct dma_chan		*chan;
>> +	dma_addr_t		addr;
>> +	dma_cookie_t		cookie;
>> +	u8			*buf;
>> +	bool			valid_buf_seg;
>> +	int			buf_offset;
>> +	u8			fifo_thresh;
>> +};
>> +
>>  struct tiadc_device {
>>  	struct ti_tscadc_dev *mfd_tscadc;
>> +	struct tiadc_dma dma;
>>  	struct mutex fifo1_lock; /* to protect fifo access */
>>  	int channels;
>> +	int total_ch_enabled;
>>  	u8 channel_line[8];
>>  	u8 channel_step[8];
>>  	int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>>  	u16 *data = adc_dev->data;
>>  
>>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>>  	for (k = 0; k < fifo1count; k = k + i) {
>>  		for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>>  			read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>>  	return IRQ_HANDLED;
>>  }
>>  
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> +	struct iio_dev *indio_dev = param;
>> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>> +	u8 *data;
>> +	int i;
>> +
>> +	data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
> 
> bool valid_buf_seg ? The buffer segment is valid or not valid? Which buffer
> segment is valid when valid_buf_seg is true?
> 
> I know what this is doing, but it is logically not correct.
> 
> Instead you could have:
> 
> int current_period; /* The period the DMA is working on */
> 
> When you start the DMA:
> dma->current_period = 0; /* We start to fill period 0 */
> 
> In here:
> 
> u8 *data = dma->buf + (dma->current_period * dma_period_size);
> 
>> +	dma->valid_buf_seg = !dma->valid_buf_seg;
> 
> /* Currently we have only two periods, so we can just */
> dma->current_period = !dma->current_period;
> /* or */
> dma->current_period = 1 - dma->current_period; /* swap the buffer ID */
> 
> I think this would make it better to follow and if later you figure that you
> want three or more periods instead of the two, it is going to be easier to add
> that.

Okay, will change this in next version patch.

> 
>> +
>> +	for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> +		iio_push_to_buffers(indio_dev, data);
>> +		data += indio_dev->scan_bytes;
>> +	}
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> +	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>> +	struct dma_async_tx_descriptor *desc;
>> +
>> +	dma->valid_buf_seg = false;
>> +	dma->fifo_thresh = FIFO1_THRESHOLD;
> 
> FIFO1_THRESHOLD is defined as 19 in the global header file, so
> if you have 2 channels enabled the trigger comes when the FIFO have 10
> samples/ch, in case of all 8 channels enabled you will round it down the
> trigger comes when you have 2 samples/ch.
> 
> Would not be better to try to use high threshold for the FIFO to reduce the
> number of DMA requests needed runtime?
> 
> It is OK as it is at the moment, but I think it would make sense to try to
> maximize the FIFO usage.

I had already tried with higher values, It works fine. Just don't want
to change the non-DMA implementation as I am not aware of any
limitations why at the first place they have used fifo threshold as 19.

> 
>> +	/*
>> +	 * Make the fifo thresh as the multiple of total number of
>> +	 * channels enabled, so make sure that that cyclic DMA period
>> +	 * length is also a multiple of total number of channels
>> +	 * enabled. This ensures that no invalid data is reported
>> +	 * to the stack via iio_push_to_buffers().
>> +	 */
>> +	dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
>> +	dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> +	/* Make sure that period length is multiple of fifo thresh level */
>> +	dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> +					      sizeof(u16));
>> +
>> +	dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> +	dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> +	desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> +					 dma->buf_offset * 2,
>> +					 dma->buf_offset, DMA_DEV_TO_MEM,
> 
> I find the buf_offset name and it's use really confusing. It should be called
> period_size, really.

Yes, will fix this.

> 
>> +					 DMA_PREP_INTERRUPT);
>> +	if (!desc)
>> +		return -EBUSY;
>> +
>> +	desc->callback = tiadc_dma_rx_complete;
>> +	desc->callback_param = indio_dev;
>> +
>> +	dma->cookie = dmaengine_submit(desc);
>> +
>> +	dma_async_issue_pending(dma->chan);
>> +
>> +	tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> +	tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> +	tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> +	return 0;
>> +}
>> +
>>  static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>>  static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	unsigned int enb = 0;
>>  	u8 bit;
>> +	u32 irq_enable;
>>  
>>  	tiadc_step_config(indio_dev);
>> -	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> +	for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>>  		enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> +		adc_dev->total_ch_enabled++;
>> +	}
>>  	adc_dev->buffer_en_ch_steps = enb;
>>  
>> +	if (dma->chan)
>> +		tiadc_start_dma(indio_dev);
>> +
>>  	am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>  
>>  	tiadc_writel(adc_dev,  REG_IRQSTATUS, IRQENB_FIFO1THRES
>>  				| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> -	tiadc_writel(adc_dev,  REG_IRQENABLE, IRQENB_FIFO1THRES
>> -				| IRQENB_FIFO1OVRRUN);
>> +
>> +	irq_enable = IRQENB_FIFO1OVRRUN;
>> +	if (!dma->chan)
>> +		irq_enable |= IRQENB_FIFO1THRES;
>> +	tiadc_writel(adc_dev,  REG_IRQENABLE, irq_enable);
>>  
>>  	return 0;
>>  }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>>  static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>>  {
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	int fifo1count, i, read;
>>  
>>  	tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>>  				IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>>  	am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>>  	adc_dev->buffer_en_ch_steps = 0;
>> +	adc_dev->total_ch_enabled = 0;
>> +	if (dma->chan) {
>> +		tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> +		dmaengine_terminate_async(dma->chan);
>> +	}
>>  
>>  	/* Flush FIFO of leftover data in the time it takes to disable adc */
>>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>>  	.driver_module = THIS_MODULE,
>>  };
>>  
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> +	return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> +			     struct tiadc_device *adc_dev)
>> +{
>> +	struct tiadc_dma	*dma = &adc_dev->dma;
>> +	dma_cap_mask_t		mask;
>> +
>> +	dma->fn = the_no_dma_filter_fn;
>> +
>> +	/* Default slave configuration parameters */
>> +	dma->conf.direction = DMA_DEV_TO_MEM;
>> +	dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> +	dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> +	dma_cap_zero(mask);
>> +	dma_cap_set(DMA_CYCLIC, mask);
>> +
>> +	/* Get a channel for RX */
>> +	dma->chan = dma_request_slave_channel_compat(mask,
>> +						     dma->fn, dma->param,
>> +						     adc_dev->mfd_tscadc->dev,
>> +						     "fifo1");
> 
> dma_request_chan()

Will fix this.

> 
>> +	if (!dma->chan)
>> +		return -ENODEV;
>> +
>> +	/* RX buffer */
>> +	dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> +				      &dma->addr, GFP_KERNEL);
>> +	if (!dma->buf)
>> +		goto err;
>> +
>> +	dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>> +
>> +	return 0;
>> +err:
>> +	dma_release_channel(dma->chan);
>> +
>> +	return -ENOMEM;
>> +}
>> +
>>  static int tiadc_parse_dt(struct platform_device *pdev,
>>  			  struct tiadc_device *adc_dev)
>>  {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>  
>>  	platform_set_drvdata(pdev, indio_dev);
>>  
>> +	err = tiadc_request_dma(pdev, adc_dev);
>> +	if (err && err != -ENODEV)
>> +		goto err_dma;
>> +
>>  	return 0;
>>  
>> +err_dma:
>> +	iio_device_unregister(indio_dev);
>>  err_buffer_unregister:
>>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>>  err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>>  {
>>  	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> +	struct tiadc_dma *dma = &adc_dev->dma;
>>  	u32 step_en;
>>  
>> +	if (dma->chan)
>> +		dma_release_channel(dma->chan);
>>  	iio_device_unregister(indio_dev);
>>  	tiadc_iio_buffered_hardware_remove(indio_dev);
>>  	tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>>  #define REG_IRQENABLE		0x02C
>>  #define REG_IRQCLR		0x030
>>  #define REG_IRQWAKEUP		0x034
>> +#define REG_DMAENABLE_SET	0x038
>> +#define REG_DMAENABLE_CLEAR	0x038
>>  #define REG_CTRL		0x040
>>  #define REG_ADCFSM		0x044
>>  #define REG_CLKDIV		0x04C
>> @@ -36,6 +38,7 @@
>>  #define REG_FIFO0THR		0xE8
>>  #define REG_FIFO1CNT		0xF0
>>  #define REG_FIFO1THR		0xF4
>> +#define REG_DMA1REQ		0xF8
>>  #define REG_FIFO0		0x100
>>  #define REG_FIFO1		0x200
>>  
>> @@ -126,6 +129,10 @@
>>  #define FIFOREAD_DATA_MASK (0xfff << 0)
>>  #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>  
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0		BIT(0)
>> +#define DMA_FIFO1		BIT(1)
>> +
>>  /* Sequencer Status */
>>  #define SEQ_STATUS BIT(5)
>>  #define CHARGE_STEP		0x11
>>
> 
> 

--
Regards
Mugunthan V N

^ permalink raw reply

* [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-specific register range for ACPI case
From: Gabriele Paoloni @ 2016-09-22 11:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160922094955.GB18637@red-moon>

Hi Lorenzo, Bjorn

> -----Original Message-----
> From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi at arm.com]
> Sent: 22 September 2016 10:50
> To: Bjorn Helgaas
> Cc: Ard Biesheuvel; Tomasz Nowicki; David Daney; Will Deacon; Catalin
> Marinas; Rafael Wysocki; Arnd Bergmann; Hanjun Guo; Sinan Kaya;
> Jayachandran C; Christopher Covington; Duc Dang; Robert Richter; Marcin
> Wojtas; Liviu Dudau; Wangyijing; Mark Salter; linux-
> pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Linaro ACPI
> Mailman List; Jon Masters; Andrea Gallo; Jeremy Linton; liudongdong
> (C); Gabriele Paoloni; Jeff Hugo; linux-acpi at vger.kernel.org; linux-
> kernel at vger.kernel.org; Rafael J. Wysocki
> Subject: Re: [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-
> specific register range for ACPI case
> 
> On Wed, Sep 21, 2016 at 01:04:57PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 21, 2016 at 03:05:49PM +0100, Lorenzo Pieralisi wrote:
> > > On Tue, Sep 20, 2016 at 02:17:44PM -0500, Bjorn Helgaas wrote:
> > > > On Tue, Sep 20, 2016 at 04:09:25PM +0100, Ard Biesheuvel wrote:
> > >
> > > [...]
> > >
> > > > > None of these platforms can be fixed entirely in software, and
> given
> > > > > that we will not be adding quirks for new broken hardware, we
> should
> > > > > ask ourselves whether having two versions of a quirk, i.e., one
> for
> > > > > broken hardware + currently shipping firmware, and one for the
> same
> > > > > broken hardware with fixed firmware is really an improvement
> over what
> > > > > has been proposed here.
> > > >
> > > > We're talking about two completely different types of quirks:
> > > >
> > > >   1) MCFG quirks to use memory-mapped config space that doesn't
> quite
> > > >      conform to the ECAM model in the PCIe spec, and
> > > >
> > > >   2) Some yet-to-be-determined method to describe address space
> > > >      consumed by a bridge.
> > > >
> > > > The first two patches of this series are a nice implementation
> for 1).
> > > > The third patch (ThunderX-specific) is one possibility for 2),
> but I
> > > > don't like it because there's no way for generic software like
> the
> > > > ACPI core to discover these resources.
> > >
> > > Ok, so basically this means that to implement (2) we need to assign
> > > some sort of _HID to these quirky PCI bridges (so that we know what
> > > device they represent and we can retrieve their _CRS). I take from
> > > this discussion that the goal is to make sure that all non-config
> > > resources have to be declared through _CRS device objects, which is
> > > fine but that requires a FW update (unless we can fabricate ACPI
> > > devices and corresponding _CRS in the kernel whenever we match a
> > > given MCFG table signature).
> >
> > All resources consumed by ACPI devices should be declared through
> > _CRS.  If you want to fabricate ACPI devices or _CRS via kernel
> > quirks, that's fine with me.  This could be triggered via MCFG
> > signature, DMI info, host bridge _HID, etc.
> 
> I think the PNP quirk approach + PNP0c02 resource put forward by Gab
> is enough.

Great thanks as we take a final decision I will ask Dogndgong to submit
another RFC based on this approach

> 
> > > We discussed this already and I think we should make a decision:
> > >
> > > http://lists.infradead.org/pipermail/linux-arm-kernel/2016-
> March/414722.html
> > >
> > > > > > I'd like to step back and come up with some understanding of
> how
> > > > > > non-broken firmware *should* deal with this issue.  Then, if
> we *do*
> > > > > > work around this particular broken firmware in the kernel, it
> would be
> > > > > > nice to do it in a way that fits in with that understanding.
> > > > > >
> > > > > > For example, if a companion ACPI device is the preferred
> solution, an
> > > > > > ACPI quirk could fabricate a device with the required
> resources.  That
> > > > > > would address the problem closer to the source and make it
> more likely
> > > > > > that the rest of the system will work correctly: /proc/iomem
> could
> > > > > > make sense, things that look at _CRS generically would work
> (e.g,
> > > > > > /sys/, an admittedly hypothetical "lsacpi", etc.)
> > > > > >
> > > > > > Hard-coding stuff in drivers is a point solution that doesn't
> provide
> > > > > > any guidance for future platforms and makes it likely that
> the hack
> > > > > > will get copied into even more drivers.
> > > > > >
> > > > >
> > > > > OK, I see. But the guidance for future platforms should be 'do
> not
> > > > > rely on quirks', and what I am arguing here is that the more we
> polish
> > > > > up this code and make it clean and reusable, the more likely it
> is
> > > > > that will end up getting abused by new broken hardware that we
> set out
> > > > > to reject entirely in the first place.
> > > > >
> > > > > So of course, if the quirk involves claiming resources, let's
> make
> > > > > sure that this occurs in the cleanest and most compliant way
> possible.
> > > > > But any factoring/reuse concerns other than for the current
> crop of
> > > > > broken hardware should be avoided imo.
> > > >
> > > > If future hardware is completely ECAM-compliant and we don't need
> any
> > > > more MCFG quirks, that would be great.
> > >
> > > Yes.
> > >
> > > > But we'll still need to describe that memory-mapped config space
> > > > somewhere.  If that's done with PNP0C02 or similar devices (as is
> done
> > > > on my x86 laptop), we'd be all set.
> > >
> > > I am not sure I understand what you mean here. Are you referring
> > > to MCFG regions reported as PNP0c02 resources through its _CRS ?
> >
> > Yes.  PCI Firmware Spec r3.0, Table 4-2, note 2 says address ranges
> > reported via MCFG or _CBA should be reserved by _CRS of a PNP0C02
> > device.
> 
> Ok, that's agreed. It goes without saying that since you are quoting
> the PCI spec, if FW fails to report MCFG regions in a PNP0c02 device
> _CRS I will consider that a FW bug.
> 
> > > IIUC PNP0C02 is a reservation mechanism, but it does not help us
> > > associate its _CRS to a specific PCI host bridge instance, right ?
> >
> > Gab proposed a hierarchy that *would* associate a PNP0C02 device with
> > a PCI bridge:
> >
> >   Device (PCI1) {
> >     Name (_HID, "HISI0080") // PCI Express Root Bridge
> >     Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
> >     Method (_CRS, 0, Serialized) { // Root complex resources
> (windows) }
> >     Device (RES0) {
> >       Name (_HID, "HISI0081") // HiSi PCIe RC config base address
> >       Name (_CID, "PNP0C02")  // Motherboard reserved resource
> >       Name (_CRS, ResourceTemplate () { ... }
> >     }
> >   }
> >
> > That's a possibility.  The PCI Firmware Spec suggests putting RES0 at
> > the root (under \_SB), but I don't know why.
> >
> > Putting it at the root means we couldn't generically associate it
> with
> > a bridge, although I could imagine something like this:
> >
> >   Device (RES1) {
> >     Name (_HID, "HISI0081") // HiSi PCIe RC config base address
> >     Name (_CID, "PNP0C02")  // Motherboard reserved resource
> >     Name (_CRS, ResourceTemplate () { ... }
> >     Method (BRDG) { "PCI1" }  // hand-wavy ASL
> >   }
> >   Device (PCI1) {
> >     Name (_HID, "HISI0080") // PCI Express Root Bridge
> >     Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
> >     Method (_CRS, 0, Serialized) { // Root complex resources
> (windows) }
> >   }
> >
> > Where you could search PNP0C02 devices for a cookie that matched the
> > host bridge.o
> 
> Ok, I am fine with both and I think we are converging, but the way
> to solve this problem has to be uniform for all ARM partners (and
> not only ARM). Two points here:
> 
> 1) Adding a device/subdevice allows people to add a _CRS reporting the
>    non-window bridge resources. Fine. It also allows people to chuck in
>    there all sorts of _DSD properties to describe their PCI host bridge
>    as it is done with DT properties (those _DSD can contain eg clocks
>    etc.), this may be tempting (so that they can reuse the same DT
>    driver and do not have to update their firmware) but I want to be
>    clear here: that must not happen. So, a subdevice with a _CRS to
>    report resources, yes, but it will stop there.
> 2) It is unclear to me how to formalize the above. People should not
>    write FW by reading the PCI mailing list, so these guidelines have
> to
>    be written, somehow. I do not want to standardize quirks, I want
>    to prevent random ACPI table content, which is different.
>    Should I report this to the ACPI spec working group ? If we do
>    not do that everyone will go solve this problem as they deem fit.
> 

Do we really need to formalize this?

As we discussed in the Linaro call at the moment we have few vendors
that need quirks and we want to avoid promoting/accepting quirks for
the future.

At the time of the call I think we decided to informally accept a set
of quirks for the current platforms and reject any other quirk coming
after a certain date/kernel version (this to be decided).

I am not sure if there is a way to document/formalize a temporary
exception from the rule...

Thanks

Gab 


> [...]
> 
> > > For FW that is immutable I really do not see what we can do apart
> > > from hardcoding the non-config resources (consumed by a bridge),
> > > somehow.
> >
> > Right.  Well, I assume you mean we should hard-code "non-window
> > resources consumed directly by a bridge".  If firmware in the field
> is
> > broken, we should work around it, and that may mean hard-coding some
> > resources.
> >
> > My point is that the hard-coding should not be buried in a driver
> > where it's invisible to the rest of the kernel.  If we hard-code it
> in
> > a quirk that adds _CRS entries, then the kernel will work just like
> it
> > would if the firmware had been correct in the first place.  The
> > resource will appear in /sys/devices/pnp*/*/resources and
> /proc/iomem,
> > and if we ever used _SRS to assign or move ACPI devices, we would
> know
> > to avoid the bridge resource.
> 
> We are in complete agreement here.
> 
> Thanks,
> Lorenzo

^ permalink raw reply

* [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-specific register range for ACPI case
From: Gabriele Paoloni @ 2016-09-22 11:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160921185920.GC20006@localhost>

Hi Bjorn

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas at kernel.org]
> Sent: 21 September 2016 19:59
> To: Gabriele Paoloni
> Cc: Ard Biesheuvel; Tomasz Nowicki; David Daney; Will Deacon; Catalin
> Marinas; Rafael Wysocki; Lorenzo Pieralisi; Arnd Bergmann; Hanjun Guo;
> Sinan Kaya; Jayachandran C; Christopher Covington; Duc Dang; Robert
> Richter; Marcin Wojtas; Liviu Dudau; Wangyijing; Mark Salter; linux-
> pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Linaro ACPI
> Mailman List; Jon Masters; Andrea Gallo; Jeremy Linton; liudongdong
> (C); Jeff Hugo; linux-acpi at vger.kernel.org; linux-
> kernel at vger.kernel.org; Rafael J. Wysocki
> Subject: Re: [PATCH V6 3/5] PCI: thunder-pem: Allow to probe PEM-
> specific register range for ACPI case
> 
> On Wed, Sep 21, 2016 at 02:10:55PM +0000, Gabriele Paoloni wrote:
> > Hi Bjorn
> >
> > [...]
> >
> >
> > >
> > > If future hardware is completely ECAM-compliant and we don't need
> any
> > > more MCFG quirks, that would be great.
> > >
> > > But we'll still need to describe that memory-mapped config space
> > > somewhere.  If that's done with PNP0C02 or similar devices (as is
> done
> > > on my x86 laptop), we'd be all set.
> > >
> > > If we need to work around firmware in the field that doesn't do
> that,
> > > one possibility is a PNP quirk along the lines of
> > > quirk_amd_mmconfig_area().
> >
> > So, if my understanding is correct, for platforms that have not been
> > shipped yet you propose to use PNP0C02 in the ACPI table in order to
> > declare a motherboard reserved resource whereas for shipped platforms
> > you propose to have a quirk along pnp_fixups in order to track the
> > resource usage even if values are hardcoded...correct?
> 
> Yes.  I'm open to alternate proposals, but x86 uses PNP0C02, and
> following existing practice seems reasonable.
> 
> > Before Tomasz came up with this patchset we had a call between the
> vendors
> > involved in this PCI quirks saga and other guys from Linaro and ARM.
> >
> > Lorenzo summarized the outcome as in the following link
> > http://lkml.iu.edu/hypermail/linux/kernel/1606.2/03344.html
> >
> > Since this quirks mechanism has been discussed for quite a long time
> now
> > IMHO it would be good to have a last call including also you (Bjorn)
> so
> > that we can all agree on what to do and we avoid changing our drivers
> again
> > and again...
> 
> I think we're converging pretty fast.  As far as I'm concerned, the
> v6 ECAM quirks implementation is perfect.  The only remaining issue is
> reporting the ECAM resources, and I haven't seen objections to using
> PNP0C02 + PNP quirks for broken firmware.
> 
> There is the question of how or whether to associate a PNP0A03 PCI
> bridge with resources from a different PNP0C02 device, but that's not
> super important.  If the hard-coded resources appear both in a quirk
> and in the PCI bridge driver, it's ugly but not the end of the world.
> We've still achieved the objective of avoiding landmines in the
> address space.

Ok got it many thanks

Gab

> 
> Bjorn

^ 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