Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix warnings for i2c-rk3x.c
From: Wolfram Sang @ 2016-09-22 17:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474543764-51135-1-git-send-email-david.wu@rock-chips.com>

On Thu, Sep 22, 2016 at 07:29:22PM +0800, David Wu wrote:
> David Wu (2):
>   i2c: rk3x: Fix sparse warning
>   i2c: rk3x: fix variable 'min_total_ns' unused warning
> 

Applied to for-next, thanks!

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160922/95673b3a/attachment.sig>

^ permalink raw reply

* [PATCH] ARM: dts: exynos: Add reboot reason support for Trats2
From: Krzysztof Kozlowski @ 2016-09-22 17:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474562915-4319-1-git-send-email-wolfgit@wiedmeyer.de>

Hi,

Thanks for the patch, it is nice that someone did this! When I saw the
syscon reboot mode driver, I though about such feature but never had
time or will to implement it. Great job!

Few comments below.

On Thu, Sep 22, 2016 at 06:48:35PM +0200, Wolfgang Wiedmeyer wrote:
> This allows to reboot the device into recovery mode and into the download
> mode of the bootloader.

Which bootloader? Probably UBoot... or Samsung stock one? Could you put
that information here?

> 
> Signed-off-by: Wolfgang Wiedmeyer <wolfgit@wiedmeyer.de>
> ---
>  arch/arm/boot/dts/exynos4412-trats2.dts | 14 ++++++++++++++
>  arch/arm/boot/dts/exynos4x12.dtsi       |  2 +-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
> index 129e973..a38d1e3 100644
> --- a/arch/arm/boot/dts/exynos4412-trats2.dts
> +++ b/arch/arm/boot/dts/exynos4412-trats2.dts
> @@ -1294,3 +1294,17 @@
>  	vtmu-supply = <&ldo10_reg>;
>  	status = "okay";
>  };
> +
> +&pmu {
> +	compatible = "syscon", "simple-mfd";
> +
> +	reboot-mode {
> +		compatible = "syscon-reboot-mode";
> +		offset = <0x80c>;
> +
> +		mode-normal	= <0x12345670>;
> +		mode-bootloader	= <0x12345671>;
> +		mode-download	= <0x12345671>;
> +		mode-recovery	= <0x12345674>;

Hmmm, how did you get these values? Are they already supported?

It would be nice to document them:
1. In Documentation/arm/Samsung/Bootloader-interface.txt
2. In header. I hate such magic numbers... you could add new header next
   to existing rockchip one:
   include/dt-bindings/soc/samsung,boot-mode.h
   (and update maintainers entry :) )


Best regards,
Krzysztof


> +	};
> +};
> diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
> index c452499..b58c14e 100644
> --- a/arch/arm/boot/dts/exynos4x12.dtsi
> +++ b/arch/arm/boot/dts/exynos4x12.dtsi
> @@ -179,7 +179,7 @@
>  			ranges;
>  			status = "disabled";
>  
> -			pmu at 10020000 {
> +			pmu: pmu at 10020000 {
>  				reg = <0x10020000 0x3000>;
>  			};
>  
> -- 
> Website: https://fossencdi.org
> OpenPGP: 0F30 D1A0 2F73 F70A 6FEE  048E 5816 A24C 1075 7FC4
> Key download: https://wiedmeyer.de/keys/ww.asc
> 

^ permalink raw reply

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

On 09/22/2016 02:05 AM, sunil.kovvuri at gmail.com wrote:
> From: Sunil Goutham <sgoutham@cavium.com>
> 
> This patch adds support for byte queue limits
> 
> Signed-off-by: Sunil Goutham <sgoutham@cavium.com>

Where is the code that calls netdev_tx_reset_queue()? This is needed in
the function that brings down the interface, did your test survive a
up/down/up sequence?

> ---
>  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:
> 


-- 
Florian

^ permalink raw reply

* [PATCH v2] musb: Export musb_root_disconnect for use in modules
From: Mark Brown @ 2016-09-22 17:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160922172842.GA23726@kroah.com>

On Thu, Sep 22, 2016 at 07:28:42PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Sep 22, 2016 at 06:15:26PM +0100, Mark Brown wrote:
> > On Thu, Sep 22, 2016 at 08:30:16AM -0500, Bin Liu wrote:

> > > Removed it from my tree, since Greg already picked it.

> > It's not showing in today's -next...

> It probably missed the cut-off, should be there tomorrow.

Ah, excellent thanks!  Down to only one build break in all*config in -next :/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160922/10a502e1/attachment.sig>

^ permalink raw reply

* [PATCH v2] musb: Export musb_root_disconnect for use in modules
From: Greg Kroah-Hartman @ 2016-09-22 17:28 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160922171526.GR7994@sirena.org.uk>

On Thu, Sep 22, 2016 at 06:15:26PM +0100, Mark Brown wrote:
> On Thu, Sep 22, 2016 at 08:30:16AM -0500, Bin Liu wrote:
> > On Wed, Sep 21, 2016 at 03:51:33PM -0500, Bin Liu wrote:
> 
> > > Applied. Thanks.
> 
> > Removed it from my tree, since Greg already picked it.
> 
> It's not showing in today's -next...

It probably missed the cut-off, should be there tomorrow.

^ permalink raw reply

* [PATCH v3 2/4] regulator: axp20x: duplicate the MFD axp20x-rsb code
From: Jean-Francois Moine @ 2016-09-22 17:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1474621107.git.moinejf@free.fr>

The axp20x rsb driver handles many different devices.
Duplicating its code in a generic regulator driver permits
to probe/remove individual devices.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/regulator/axp-regulator.c | 39 +++++++++++++++++++++++++++++++++++++++
 drivers/regulator/axp-regulator.h |  6 ++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/regulator/axp-regulator.c b/drivers/regulator/axp-regulator.c
index 0d7adb6..17943fb 100644
--- a/drivers/regulator/axp-regulator.c
+++ b/drivers/regulator/axp-regulator.c
@@ -303,6 +303,45 @@ int axp_regulator_create(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(axp_regulator_create);
 
+/* probe/remove RSB devices */
+int axp_rsb_probe(struct sunxi_rsb_device *rdev,
+		  struct axp20x_dev *axp20x,
+		  const struct axp_cfg *axp_cfg)
+{
+	int ret;
+
+	axp20x->dev = &rdev->dev;
+	axp20x->irq = rdev->irq;
+	dev_set_drvdata(&rdev->dev, axp20x);
+
+	ret = axp20x_match_device(axp20x);
+	if (ret)
+		return ret;
+
+	axp20x->regmap = devm_regmap_init_sunxi_rsb(rdev,
+						    axp20x->regmap_cfg);
+	if (IS_ERR(axp20x->regmap)) {
+		ret = PTR_ERR(axp20x->regmap);
+		dev_err(&rdev->dev, "regmap init failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = axp20x_device_probe(axp20x);
+	if (ret < 0)
+		return ret;
+
+	return axp_regulator_create(&rdev->dev, axp_cfg);
+}
+EXPORT_SYMBOL_GPL(axp_rsb_probe);
+
+int axp_rsb_remove(struct sunxi_rsb_device *rdev)
+{
+	struct axp20x_dev *axp20x = sunxi_rsb_device_get_drvdata(rdev);
+
+	return axp20x_device_remove(axp20x);
+}
+EXPORT_SYMBOL_GPL(axp_rsb_remove);
+
 MODULE_AUTHOR("Carlo Caione <carlo@caione.org>");
 MODULE_DESCRIPTION("Regulator Module for AXP PMIC");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/regulator/axp-regulator.h b/drivers/regulator/axp-regulator.h
index 0adf1b0..085eaa0 100644
--- a/drivers/regulator/axp-regulator.h
+++ b/drivers/regulator/axp-regulator.h
@@ -124,4 +124,10 @@ struct axp_cfg {
 int axp_regulator_create(struct device *dev,
 			 const struct axp_cfg *axp_cfg);
 
+struct sunxi_rsb_device;
+int axp_rsb_probe(struct sunxi_rsb_device *rdev,
+		  struct axp20x_dev *axp20x,
+		  const struct axp_cfg *axp_cfg);
+int axp_rsb_remove(struct sunxi_rsb_device *rdev);
+
 #endif /* __AXP_REGULATOR_H__ */
-- 
2.10.0

^ permalink raw reply related

* [PATCH v2] musb: Export musb_root_disconnect for use in modules
From: Mark Brown @ 2016-09-22 17:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160922133016.GE3970@uda0271908>

On Thu, Sep 22, 2016 at 08:30:16AM -0500, Bin Liu wrote:
> On Wed, Sep 21, 2016 at 03:51:33PM -0500, Bin Liu wrote:

> > Applied. Thanks.

> Removed it from my tree, since Greg already picked it.

It's not showing in today's -next...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160922/22161643/attachment-0001.sig>

^ permalink raw reply

* [PATCH] PCI: rockchip: fix uninitialized variable use
From: Brian Norris @ 2016-09-22 17:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160922094018.2138640-1-arnd@arndb.de>

On Thu, Sep 22, 2016 at 11:39:59AM +0200, Arnd Bergmann wrote:
> 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:

I feel like we've fixed this one before, but then it's been refactored
many times along the way...

Reviewed-by: Brian Norris <briannorris@chromium.org>

^ permalink raw reply

* [PATCH] arm64: Call numa_store_cpu_info() earlier.
From: Catalin Marinas @ 2016-09-22 17:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474397195-16520-1-git-send-email-ddaney.cavm@gmail.com>

On Tue, Sep 20, 2016 at 11:46:35AM -0700, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> The wq_numa_init() function makes a private CPU to node map by calling
> cpu_to_node() early in the boot process, before the non-boot CPUs are
> brought online.  Since the default implementation of cpu_to_node()
> returns zero for CPUs that have never been brought online, the
> workqueue system's view is that *all* CPUs are on node zero.
> 
> When the unbound workqueue for a non-zero node is created, the
> tsk_cpus_allowed() for the worker threads is the empty set because
> there are, in the view of the workqueue system, no CPUs on non-zero
> nodes.  The code in try_to_wake_up() using this empty cpumask ends up
> using the cpumask empty set value of NR_CPUS as an index into the
> per-CPU area pointer array, and gets garbage as it is one past the end
> of the array.  This results in:

Queued for 4.8. Thanks.

-- 
Catalin

^ permalink raw reply

* [PATCH v3 1/4] regulator: axp20x: move device independant parts to new files
From: Jean-Francois Moine @ 2016-09-22 17:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1474621107.git.moinejf@free.fr>

The axp20x driver contains device specific and device independant parts.
This patch moves the independant parts to new .c/.h files.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/regulator/Makefile           |   2 +-
 drivers/regulator/axp-regulator.c    | 308 ++++++++++++++++++++++++++
 drivers/regulator/axp-regulator.h    | 127 +++++++++++
 drivers/regulator/axp20x-regulator.c | 415 +++--------------------------------
 4 files changed, 464 insertions(+), 388 deletions(-)
 create mode 100644 drivers/regulator/axp-regulator.c
 create mode 100644 drivers/regulator/axp-regulator.h

diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 2142a5d..225a026 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -21,7 +21,7 @@ obj-$(CONFIG_REGULATOR_ANATOP) += anatop-regulator.o
 obj-$(CONFIG_REGULATOR_ARIZONA) += arizona-micsupp.o arizona-ldo1.o
 obj-$(CONFIG_REGULATOR_AS3711) += as3711-regulator.o
 obj-$(CONFIG_REGULATOR_AS3722) += as3722-regulator.o
-obj-$(CONFIG_REGULATOR_AXP20X) += axp20x-regulator.o
+obj-$(CONFIG_REGULATOR_AXP20X) += axp20x-regulator.o axp-regulator.o
 obj-$(CONFIG_REGULATOR_BCM590XX) += bcm590xx-regulator.o
 obj-$(CONFIG_REGULATOR_DA903X)	+= da903x.o
 obj-$(CONFIG_REGULATOR_DA9052)	+= da9052-regulator.o
diff --git a/drivers/regulator/axp-regulator.c b/drivers/regulator/axp-regulator.c
new file mode 100644
index 0000000..0d7adb6
--- /dev/null
+++ b/drivers/regulator/axp-regulator.c
@@ -0,0 +1,308 @@
+/*
+ * AXP regulators driver
+ *
+ * Copyright (C) 2016 Jean-Francois Moine <moinejf@free.fr>
+ * Copyright (C) 2013 Carlo Caione <carlo@caione.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/mfd/axp20x.h>
+#include <linux/sunxi-rsb.h>
+
+#include "axp-regulator.h"
+
+#define AXP20X_WORKMODE_DCDC2_MASK	BIT(2)
+#define AXP20X_WORKMODE_DCDC3_MASK	BIT(1)
+#define AXP22X_WORKMODE_DCDCX_MASK(x)	BIT(x)
+
+#define AXP20X_FREQ_DCDC_MASK	0x0f
+
+#define AXP22X_MISC_N_VBUSEN_FUNC	BIT(4)
+
+const struct regulator_ops axp_ops_fixed = {
+	.list_voltage		= regulator_list_voltage_linear,
+};
+EXPORT_SYMBOL_GPL(axp_ops_fixed);
+
+const struct regulator_ops axp_ops_range = {
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.list_voltage		= regulator_list_voltage_linear_range,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= regulator_is_enabled_regmap,
+};
+EXPORT_SYMBOL_GPL(axp_ops_range);
+
+const struct regulator_ops axp_ops = {
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.list_voltage		= regulator_list_voltage_linear,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= regulator_is_enabled_regmap,
+};
+EXPORT_SYMBOL_GPL(axp_ops);
+
+const struct regulator_ops axp_ops_sw = {
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= regulator_is_enabled_regmap,
+};
+EXPORT_SYMBOL_GPL(axp_ops_sw);
+
+static const struct regulator_desc axp22x_drivevbus_regulator = {
+	.name		= "drivevbus",
+	.supply_name	= "drivevbus",
+	.of_match	= of_match_ptr("drivevbus"),
+	.regulators_node = of_match_ptr("regulators"),
+	.type		= REGULATOR_VOLTAGE,
+	.owner		= THIS_MODULE,
+	.enable_reg	= AXP20X_VBUS_IPSOUT_MGMT,
+	.enable_mask	= BIT(2),
+	.ops		= &axp_ops_sw,
+};
+
+static int axp_set_dcdc_freq(struct device *dev,
+					u32 dcdcfreq)
+{
+	struct axp20x_dev *axp20x = dev_get_drvdata(dev);
+	unsigned int reg = AXP20X_DCDC_FREQ;
+	u32 min, max, def, step;
+
+	switch (axp20x->variant) {
+	case AXP202_ID:
+	case AXP209_ID:
+		min = 750;
+		max = 1875;
+		def = 1500;
+		step = 75;
+		break;
+	case AXP806_ID:
+		/*
+		 * AXP806 DCDC work frequency setting has the same range and
+		 * step as AXP22X, but at a different register.
+		 * Fall through to the check below.
+		 * (See include/linux/mfd/axp20x.h)
+		 */
+		reg = AXP806_DCDC_FREQ_CTRL;
+	case AXP221_ID:
+	case AXP223_ID:
+	case AXP809_ID:
+		min = 1800;
+		max = 4050;
+		def = 3000;
+		step = 150;
+		break;
+	default:
+		dev_err(dev,
+			"Setting DCDC frequency for unsupported AXP variant\n");
+		return -EINVAL;
+	}
+
+	if (dcdcfreq == 0)
+		dcdcfreq = def;
+
+	if (dcdcfreq < min) {
+		dcdcfreq = min;
+		dev_warn(dev, "DCDC frequency too low. Set to %ukHz\n",
+			 min);
+	}
+
+	if (dcdcfreq > max) {
+		dcdcfreq = max;
+		dev_warn(dev, "DCDC frequency too high. Set to %ukHz\n",
+			 max);
+	}
+
+	dcdcfreq = (dcdcfreq - min) / step;
+
+	return regmap_update_bits(axp20x->regmap, reg,
+				  AXP20X_FREQ_DCDC_MASK, dcdcfreq);
+}
+
+static int axp_regulator_parse_dt(struct device *dev)
+{
+	struct device_node *np, *regulators;
+	int ret;
+	u32 dcdcfreq = 0;
+
+	np = of_node_get(dev->of_node);
+	if (!np)
+		return 0;
+
+	regulators = of_get_child_by_name(np, "regulators");
+	if (!regulators) {
+		dev_warn(dev, "regulators node not found\n");
+	} else {
+		of_property_read_u32(regulators, "x-powers,dcdc-freq", &dcdcfreq);
+		ret = axp_set_dcdc_freq(dev, dcdcfreq);
+		if (ret < 0) {
+			dev_err(dev, "Error setting dcdc frequency: %d\n", ret);
+			return ret;
+		}
+
+		of_node_put(regulators);
+	}
+
+	return 0;
+}
+
+static int axp_set_dcdc_workmode(struct regulator_dev *rdev,
+				int id, u32 workmode)
+{
+	struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
+	unsigned int reg = AXP20X_DCDC_MODE;
+	unsigned int mask;
+
+	switch (axp20x->variant) {
+	case AXP202_ID:
+	case AXP209_ID:
+		if ((id != AXP20X_DCDC2) && (id != AXP20X_DCDC3))
+			return -EINVAL;
+
+		mask = AXP20X_WORKMODE_DCDC2_MASK;
+		if (id == AXP20X_DCDC3)
+			mask = AXP20X_WORKMODE_DCDC3_MASK;
+
+		workmode <<= ffs(mask) - 1;
+		break;
+
+	case AXP806_ID:
+		reg = AXP806_DCDC_MODE_CTRL2;
+		/*
+		 * AXP806 DCDC regulator IDs have the same range as AXP22X.
+		 * Fall through to the check below.
+		 * (See include/linux/mfd/axp20x.h)
+		 */
+	case AXP221_ID:
+	case AXP223_ID:
+	case AXP809_ID:
+		if (id < AXP22X_DCDC1 || id > AXP22X_DCDC5)
+			return -EINVAL;
+
+		mask = AXP22X_WORKMODE_DCDCX_MASK(id - AXP22X_DCDC1);
+		workmode <<= id - AXP22X_DCDC1;
+		break;
+
+	default:
+		/* should not happen */
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(rdev->regmap, reg, mask, workmode);
+}
+
+/* create the regulators */
+int axp_regulator_create(struct device *dev,
+			 const struct axp_cfg *axp_cfg)
+{
+	struct regulator_dev *rdev;
+	struct axp20x_dev *axp20x = dev_get_drvdata(dev);
+	struct regulator_config config = {
+		.dev = dev,
+		.regmap = axp20x->regmap,
+		.driver_data = axp20x,
+	};
+	int ret, i;
+	u32 workmode;
+	const char *dcdc1_name = NULL;
+	const char *dcdc5_name = NULL;
+
+	/* This only sets the dcdc freq. Ignore any errors */
+	axp_regulator_parse_dt(dev);
+
+	for (i = 0; i < axp_cfg->nregulators; i++) {
+		const struct regulator_desc *desc = &axp_cfg->regulators[i];
+		struct regulator_desc *new_desc;
+
+		if (axp_cfg->skip_bitmap & (1 << i))
+			continue;
+
+		/*
+		 * Regulators DC1SW and DC5LDO are connected internally,
+		 * so we have to handle their supply names separately.
+		 *
+		 * We always register the regulators in proper sequence,
+		 * so the supply names are correctly read. See the last
+		 * part of this loop to see where we save the DT defined
+		 * name.
+		 */
+		if (i == axp_cfg->dc1sw_ix && dcdc1_name) {
+			new_desc = devm_kzalloc(dev, sizeof(*desc),
+						GFP_KERNEL);
+			*new_desc = *desc;
+			new_desc->supply_name = dcdc1_name;
+			desc = new_desc;
+		}
+
+		if (i == axp_cfg->dc5ldo_ix && dcdc5_name) {
+			new_desc = devm_kzalloc(dev, sizeof(*desc),
+						GFP_KERNEL);
+			*new_desc = *desc;
+			new_desc->supply_name = dcdc5_name;
+			desc = new_desc;
+		}
+
+		rdev = devm_regulator_register(dev, desc, &config);
+		if (IS_ERR(rdev)) {
+			dev_err(dev, "Failed to register %s\n",
+				axp_cfg->regulators[i].name);
+
+			return PTR_ERR(rdev);
+		}
+
+		ret = of_property_read_u32(rdev->dev.of_node,
+					   "x-powers,dcdc-workmode",
+					   &workmode);
+		if (!ret) {
+			if (axp_set_dcdc_workmode(rdev, i, workmode))
+				dev_err(dev, "Failed to set workmode on %s\n",
+					rdev->desc->name);
+		}
+
+		/*
+		 * Save AXP22X DCDC1 / DCDC5 regulator names for later.
+		 */
+		if (i == axp_cfg->dcdc1_ix)
+			of_property_read_string(rdev->dev.of_node,
+						"regulator-name",
+						&dcdc1_name);
+		if (i == axp_cfg->dcdc5_ix)
+			of_property_read_string(rdev->dev.of_node,
+						"regulator-name",
+						&dcdc5_name);
+	}
+
+	if (axp_cfg->drivevbus) {
+		/* Change N_VBUSEN sense pin to DRIVEVBUS output pin */
+		regmap_update_bits(axp20x->regmap, AXP20X_OVER_TMP,
+				   AXP22X_MISC_N_VBUSEN_FUNC, 0);
+		rdev = devm_regulator_register(dev,
+					       &axp22x_drivevbus_regulator,
+					       &config);
+		if (IS_ERR(rdev)) {
+			dev_err(dev, "Failed to register drivevbus\n");
+			return PTR_ERR(rdev);
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(axp_regulator_create);
+
+MODULE_AUTHOR("Carlo Caione <carlo@caione.org>");
+MODULE_DESCRIPTION("Regulator Module for AXP PMIC");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/regulator/axp-regulator.h b/drivers/regulator/axp-regulator.h
new file mode 100644
index 0000000..0adf1b0
--- /dev/null
+++ b/drivers/regulator/axp-regulator.h
@@ -0,0 +1,127 @@
+#ifndef __AXP_REGULATOR_H__
+#define __AXP_REGULATOR_H__
+/*
+ * Copyright (C) 2016 Jean-Francois Moine <moinejf@free.fr>
+ *
+ * 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.
+ */
+
+#define AXP20X_IO_ENABLED		0x03
+#define AXP20X_IO_DISABLED		0x07
+
+#define AXP22X_IO_ENABLED		0x03
+#define AXP22X_IO_DISABLED		0x04
+
+#define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
+		    _vmask, _ereg, _emask, _enable_val, _disable_val)		\
+	[_family##_##_id] = {							\
+		.name		= (_match),					\
+		.supply_name	= (_supply),					\
+		.of_match	= of_match_ptr(_match),				\
+		.regulators_node = of_match_ptr("regulators"),			\
+		.type		= REGULATOR_VOLTAGE,				\
+		.id		= _family##_##_id,				\
+		.n_voltages	= (((_max) - (_min)) / (_step) + 1),		\
+		.owner		= THIS_MODULE,					\
+		.min_uV		= (_min) * 1000,				\
+		.uV_step	= (_step) * 1000,				\
+		.vsel_reg	= (_vreg),					\
+		.vsel_mask	= (_vmask),					\
+		.enable_reg	= (_ereg),					\
+		.enable_mask	= (_emask),					\
+		.enable_val	= (_enable_val),				\
+		.disable_val	= (_disable_val),				\
+		.ops		= &axp_ops,					\
+	}
+
+#define AXP_DESC(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
+		 _vmask, _ereg, _emask) 					\
+	[_family##_##_id] = {							\
+		.name		= (_match),					\
+		.supply_name	= (_supply),					\
+		.of_match	= of_match_ptr(_match),				\
+		.regulators_node = of_match_ptr("regulators"),			\
+		.type		= REGULATOR_VOLTAGE,				\
+		.id		= _family##_##_id,				\
+		.n_voltages	= (((_max) - (_min)) / (_step) + 1),		\
+		.owner		= THIS_MODULE,					\
+		.min_uV		= (_min) * 1000,				\
+		.uV_step	= (_step) * 1000,				\
+		.vsel_reg	= (_vreg),					\
+		.vsel_mask	= (_vmask),					\
+		.enable_reg	= (_ereg),					\
+		.enable_mask	= (_emask),					\
+		.ops		= &axp_ops,					\
+	}
+
+#define AXP_DESC_SW(_family, _id, _match, _supply, _ereg, _emask)		\
+	[_family##_##_id] = {							\
+		.name		= (_match),					\
+		.supply_name	= (_supply),					\
+		.of_match	= of_match_ptr(_match),				\
+		.regulators_node = of_match_ptr("regulators"),			\
+		.type		= REGULATOR_VOLTAGE,				\
+		.id		= _family##_##_id,				\
+		.owner		= THIS_MODULE,					\
+		.enable_reg	= (_ereg),					\
+		.enable_mask	= (_emask),					\
+		.ops		= &axp_ops_sw,					\
+	}
+
+#define AXP_DESC_FIXED(_family, _id, _match, _supply, _volt)			\
+	[_family##_##_id] = {							\
+		.name		= (_match),					\
+		.supply_name	= (_supply),					\
+		.of_match	= of_match_ptr(_match),				\
+		.regulators_node = of_match_ptr("regulators"),			\
+		.type		= REGULATOR_VOLTAGE,				\
+		.id		= _family##_##_id,				\
+		.n_voltages	= 1,						\
+		.owner		= THIS_MODULE,					\
+		.min_uV		= (_volt) * 1000,				\
+		.ops		= &axp_ops_fixed				\
+	}
+
+#define AXP_DESC_RANGES(_family, _id, _match, _supply, _ranges, _n_voltages,	\
+			_vreg, _vmask, _ereg, _emask)				\
+	[_family##_##_id] = {							\
+		.name		= (_match),					\
+		.supply_name	= (_supply),					\
+		.of_match	= of_match_ptr(_match),				\
+		.regulators_node = of_match_ptr("regulators"),			\
+		.type		= REGULATOR_VOLTAGE,				\
+		.id		= _family##_##_id,				\
+		.n_voltages	= (_n_voltages),				\
+		.owner		= THIS_MODULE,					\
+		.vsel_reg	= (_vreg),					\
+		.vsel_mask	= (_vmask),					\
+		.enable_reg	= (_ereg),					\
+		.enable_mask	= (_emask),					\
+		.linear_ranges	= (_ranges),					\
+		.n_linear_ranges = ARRAY_SIZE(_ranges),				\
+		.ops		= &axp_ops_range,				\
+	}
+
+extern const struct regulator_ops axp_ops;
+extern const struct regulator_ops axp_ops_fixed;
+extern const struct regulator_ops axp_ops_range;
+extern const struct regulator_ops axp_ops_sw;
+
+struct axp_cfg {
+	const struct regulator_desc *regulators;
+	u8 nregulators;
+	s8 dcdc1_ix;
+	s8 dcdc5_ix;
+	s8 dc1sw_ix;
+	s8 dc5ldo_ix;
+	u32 skip_bitmap;
+	bool drivevbus;
+};
+
+int axp_regulator_create(struct device *dev,
+			 const struct axp_cfg *axp_cfg);
+
+#endif /* __AXP_REGULATOR_H__ */
diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c
index 244ddc3..9dd9ca3 100644
--- a/drivers/regulator/axp20x-regulator.c
+++ b/drivers/regulator/axp20x-regulator.c
@@ -24,137 +24,7 @@
 #include <linux/regulator/driver.h>
 #include <linux/regulator/of_regulator.h>
 
-#define AXP20X_IO_ENABLED		0x03
-#define AXP20X_IO_DISABLED		0x07
-
-#define AXP22X_IO_ENABLED		0x03
-#define AXP22X_IO_DISABLED		0x04
-
-#define AXP20X_WORKMODE_DCDC2_MASK	BIT(2)
-#define AXP20X_WORKMODE_DCDC3_MASK	BIT(1)
-#define AXP22X_WORKMODE_DCDCX_MASK(x)	BIT(x)
-
-#define AXP20X_FREQ_DCDC_MASK		0x0f
-
-#define AXP22X_MISC_N_VBUSEN_FUNC	BIT(4)
-
-#define AXP_DESC_IO(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
-		    _vmask, _ereg, _emask, _enable_val, _disable_val)		\
-	[_family##_##_id] = {							\
-		.name		= (_match),					\
-		.supply_name	= (_supply),					\
-		.of_match	= of_match_ptr(_match),				\
-		.regulators_node = of_match_ptr("regulators"),			\
-		.type		= REGULATOR_VOLTAGE,				\
-		.id		= _family##_##_id,				\
-		.n_voltages	= (((_max) - (_min)) / (_step) + 1),		\
-		.owner		= THIS_MODULE,					\
-		.min_uV		= (_min) * 1000,				\
-		.uV_step	= (_step) * 1000,				\
-		.vsel_reg	= (_vreg),					\
-		.vsel_mask	= (_vmask),					\
-		.enable_reg	= (_ereg),					\
-		.enable_mask	= (_emask),					\
-		.enable_val	= (_enable_val),				\
-		.disable_val	= (_disable_val),				\
-		.ops		= &axp20x_ops,					\
-	}
-
-#define AXP_DESC(_family, _id, _match, _supply, _min, _max, _step, _vreg,	\
-		 _vmask, _ereg, _emask) 					\
-	[_family##_##_id] = {							\
-		.name		= (_match),					\
-		.supply_name	= (_supply),					\
-		.of_match	= of_match_ptr(_match),				\
-		.regulators_node = of_match_ptr("regulators"),			\
-		.type		= REGULATOR_VOLTAGE,				\
-		.id		= _family##_##_id,				\
-		.n_voltages	= (((_max) - (_min)) / (_step) + 1),		\
-		.owner		= THIS_MODULE,					\
-		.min_uV		= (_min) * 1000,				\
-		.uV_step	= (_step) * 1000,				\
-		.vsel_reg	= (_vreg),					\
-		.vsel_mask	= (_vmask),					\
-		.enable_reg	= (_ereg),					\
-		.enable_mask	= (_emask),					\
-		.ops		= &axp20x_ops,					\
-	}
-
-#define AXP_DESC_SW(_family, _id, _match, _supply, _ereg, _emask)		\
-	[_family##_##_id] = {							\
-		.name		= (_match),					\
-		.supply_name	= (_supply),					\
-		.of_match	= of_match_ptr(_match),				\
-		.regulators_node = of_match_ptr("regulators"),			\
-		.type		= REGULATOR_VOLTAGE,				\
-		.id		= _family##_##_id,				\
-		.owner		= THIS_MODULE,					\
-		.enable_reg	= (_ereg),					\
-		.enable_mask	= (_emask),					\
-		.ops		= &axp20x_ops_sw,				\
-	}
-
-#define AXP_DESC_FIXED(_family, _id, _match, _supply, _volt)			\
-	[_family##_##_id] = {							\
-		.name		= (_match),					\
-		.supply_name	= (_supply),					\
-		.of_match	= of_match_ptr(_match),				\
-		.regulators_node = of_match_ptr("regulators"),			\
-		.type		= REGULATOR_VOLTAGE,				\
-		.id		= _family##_##_id,				\
-		.n_voltages	= 1,						\
-		.owner		= THIS_MODULE,					\
-		.min_uV		= (_volt) * 1000,				\
-		.ops		= &axp20x_ops_fixed				\
-	}
-
-#define AXP_DESC_RANGES(_family, _id, _match, _supply, _ranges, _n_voltages,	\
-			_vreg, _vmask, _ereg, _emask)				\
-	[_family##_##_id] = {							\
-		.name		= (_match),					\
-		.supply_name	= (_supply),					\
-		.of_match	= of_match_ptr(_match),				\
-		.regulators_node = of_match_ptr("regulators"),			\
-		.type		= REGULATOR_VOLTAGE,				\
-		.id		= _family##_##_id,				\
-		.n_voltages	= (_n_voltages),				\
-		.owner		= THIS_MODULE,					\
-		.vsel_reg	= (_vreg),					\
-		.vsel_mask	= (_vmask),					\
-		.enable_reg	= (_ereg),					\
-		.enable_mask	= (_emask),					\
-		.linear_ranges	= (_ranges),					\
-		.n_linear_ranges = ARRAY_SIZE(_ranges),				\
-		.ops		= &axp20x_ops_range,				\
-	}
-
-static struct regulator_ops axp20x_ops_fixed = {
-	.list_voltage		= regulator_list_voltage_linear,
-};
-
-static struct regulator_ops axp20x_ops_range = {
-	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
-	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
-	.list_voltage		= regulator_list_voltage_linear_range,
-	.enable			= regulator_enable_regmap,
-	.disable		= regulator_disable_regmap,
-	.is_enabled		= regulator_is_enabled_regmap,
-};
-
-static struct regulator_ops axp20x_ops = {
-	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
-	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
-	.list_voltage		= regulator_list_voltage_linear,
-	.enable			= regulator_enable_regmap,
-	.disable		= regulator_disable_regmap,
-	.is_enabled		= regulator_is_enabled_regmap,
-};
-
-static struct regulator_ops axp20x_ops_sw = {
-	.enable			= regulator_enable_regmap,
-	.disable		= regulator_disable_regmap,
-	.is_enabled		= regulator_is_enabled_regmap,
-};
+#include "axp-regulator.h"
 
 static const struct regulator_linear_range axp20x_ldo4_ranges[] = {
 	REGULATOR_LINEAR_RANGE(1250000, 0x0, 0x0, 0),
@@ -232,18 +102,6 @@ static const struct regulator_desc axp22x_regulators[] = {
 	AXP_DESC_FIXED(AXP22X, RTC_LDO, "rtc_ldo", "ips", 3000),
 };
 
-static const struct regulator_desc axp22x_drivevbus_regulator = {
-	.name		= "drivevbus",
-	.supply_name	= "drivevbus",
-	.of_match	= of_match_ptr("drivevbus"),
-	.regulators_node = of_match_ptr("regulators"),
-	.type		= REGULATOR_VOLTAGE,
-	.owner		= THIS_MODULE,
-	.enable_reg	= AXP20X_VBUS_IPSOUT_MGMT,
-	.enable_mask	= BIT(2),
-	.ops		= &axp20x_ops_sw,
-};
-
 static const struct regulator_linear_range axp806_dcdca_ranges[] = {
 	REGULATOR_LINEAR_RANGE(600000, 0x0, 0x32, 10000),
 	REGULATOR_LINEAR_RANGE(1120000, 0x33, 0x47, 20000),
@@ -347,135 +205,6 @@ static const struct regulator_desc axp809_regulators[] = {
 	AXP_DESC_SW(AXP809, SW, "sw", "swin", AXP22X_PWR_OUT_CTRL2, BIT(6)),
 };
 
-static int axp20x_set_dcdc_freq(struct device *dev, u32 dcdcfreq)
-{
-	struct axp20x_dev *axp20x = dev_get_drvdata(dev);
-	unsigned int reg = AXP20X_DCDC_FREQ;
-	u32 min, max, def, step;
-
-	switch (axp20x->variant) {
-	case AXP202_ID:
-	case AXP209_ID:
-		min = 750;
-		max = 1875;
-		def = 1500;
-		step = 75;
-		break;
-	case AXP806_ID:
-		/*
-		 * AXP806 DCDC work frequency setting has the same range and
-		 * step as AXP22X, but at a different register.
-		 * Fall through to the check below.
-		 * (See include/linux/mfd/axp20x.h)
-		 */
-		reg = AXP806_DCDC_FREQ_CTRL;
-	case AXP221_ID:
-	case AXP223_ID:
-	case AXP809_ID:
-		min = 1800;
-		max = 4050;
-		def = 3000;
-		step = 150;
-		break;
-	default:
-		dev_err(dev,
-			"Setting DCDC frequency for unsupported AXP variant\n");
-		return -EINVAL;
-	}
-
-	if (dcdcfreq == 0)
-		dcdcfreq = def;
-
-	if (dcdcfreq < min) {
-		dcdcfreq = min;
-		dev_warn(dev, "DCDC frequency too low. Set to %ukHz\n",
-			 min);
-	}
-
-	if (dcdcfreq > max) {
-		dcdcfreq = max;
-		dev_warn(dev, "DCDC frequency too high. Set to %ukHz\n",
-			 max);
-	}
-
-	dcdcfreq = (dcdcfreq - min) / step;
-
-	return regmap_update_bits(axp20x->regmap, reg,
-				  AXP20X_FREQ_DCDC_MASK, dcdcfreq);
-}
-
-static int axp20x_regulator_parse_dt(struct device *dev)
-{
-	struct device_node *np, *regulators;
-	int ret;
-	u32 dcdcfreq = 0;
-
-	np = of_node_get(dev->of_node);
-	if (!np)
-		return 0;
-
-	regulators = of_get_child_by_name(np, "regulators");
-	if (!regulators) {
-		dev_warn(dev, "regulators node not found\n");
-	} else {
-		of_property_read_u32(regulators, "x-powers,dcdc-freq", &dcdcfreq);
-		ret = axp20x_set_dcdc_freq(dev, dcdcfreq);
-		if (ret < 0) {
-			dev_err(dev, "Error setting dcdc frequency: %d\n", ret);
-			return ret;
-		}
-
-		of_node_put(regulators);
-	}
-
-	return 0;
-}
-
-static int axp20x_set_dcdc_workmode(struct regulator_dev *rdev, int id, u32 workmode)
-{
-	struct axp20x_dev *axp20x = rdev_get_drvdata(rdev);
-	unsigned int reg = AXP20X_DCDC_MODE;
-	unsigned int mask;
-
-	switch (axp20x->variant) {
-	case AXP202_ID:
-	case AXP209_ID:
-		if ((id != AXP20X_DCDC2) && (id != AXP20X_DCDC3))
-			return -EINVAL;
-
-		mask = AXP20X_WORKMODE_DCDC2_MASK;
-		if (id == AXP20X_DCDC3)
-			mask = AXP20X_WORKMODE_DCDC3_MASK;
-
-		workmode <<= ffs(mask) - 1;
-		break;
-
-	case AXP806_ID:
-		reg = AXP806_DCDC_MODE_CTRL2;
-		/*
-		 * AXP806 DCDC regulator IDs have the same range as AXP22X.
-		 * Fall through to the check below.
-		 * (See include/linux/mfd/axp20x.h)
-		 */
-	case AXP221_ID:
-	case AXP223_ID:
-	case AXP809_ID:
-		if (id < AXP22X_DCDC1 || id > AXP22X_DCDC5)
-			return -EINVAL;
-
-		mask = AXP22X_WORKMODE_DCDCX_MASK(id - AXP22X_DCDC1);
-		workmode <<= id - AXP22X_DCDC1;
-		break;
-
-	default:
-		/* should not happen */
-		WARN_ON(1);
-		return -EINVAL;
-	}
-
-	return regmap_update_bits(rdev->regmap, reg, mask, workmode);
-}
-
 /*
  * This function checks which regulators are part of poly-phase
  * output setups based on the registers settings.
@@ -500,60 +229,51 @@ static u32 axp20x_polyphase_slave(struct axp20x_dev *axp20x)
 static int axp20x_regulator_probe(struct platform_device *pdev)
 {
 	struct device *dev = pdev->dev.parent;
-	struct regulator_dev *rdev;
 	struct axp20x_dev *axp20x = dev_get_drvdata(dev);
-	const struct regulator_desc *regulators;
-	struct regulator_config config = {
-		.dev = dev,
-		.regmap = axp20x->regmap,
-		.driver_data = axp20x,
-	};
-	int ret, i, nregulators;
-	u32 workmode;
-	const char *dcdc1_name = axp22x_regulators[AXP22X_DCDC1].name;
-	const char *dcdc5_name = axp22x_regulators[AXP22X_DCDC5].name;
-	s8 dcdc1_ix = -1;
-	s8 dcdc5_ix = -1;
-	s8 dc1sw_ix = -1;
-	s8 dc5ldo_ix = -1;
-	bool drivevbus = false;
-	u32 skip_bitmap = 0;
+	struct axp_cfg axp_cfg;
+
+	axp_cfg.dcdc1_ix = -1;
+	axp_cfg.dcdc5_ix = -1;
+	axp_cfg.dc1sw_ix = -1;
+	axp_cfg.dc5ldo_ix = -1;
+	axp_cfg.drivevbus = false;
+	axp_cfg.skip_bitmap = 0;
 
 	switch (axp20x->variant) {
 	case AXP202_ID:
 	case AXP209_ID:
-		regulators = axp20x_regulators;
-		nregulators = AXP20X_REG_ID_MAX;
+		axp_cfg.regulators = axp20x_regulators;
+		axp_cfg.nregulators = AXP20X_REG_ID_MAX;
 		break;
 	case AXP221_ID:
 	case AXP223_ID:
-		regulators = axp22x_regulators;
-		nregulators = AXP22X_REG_ID_MAX;
-		dcdc1_ix = AXP22X_DCDC1;
-		dcdc5_ix = AXP22X_DCDC5;
-		dc1sw_ix = AXP22X_DC1SW;
-		dc5ldo_ix = AXP22X_DC5LDO;
-		drivevbus = of_property_read_bool(dev->of_node,
+		axp_cfg.regulators = axp22x_regulators;
+		axp_cfg.nregulators = AXP22X_REG_ID_MAX;
+		axp_cfg.dcdc1_ix = AXP22X_DCDC1;
+		axp_cfg.dcdc5_ix = AXP22X_DCDC5;
+		axp_cfg.dc1sw_ix = AXP22X_DC1SW;
+		axp_cfg.dc5ldo_ix = AXP22X_DC5LDO;
+		axp_cfg.drivevbus = of_property_read_bool(dev->of_node,
 						  "x-powers,drive-vbus-en");
 		break;
 	case AXP806_ID:
-		regulators = axp806_regulators;
-		nregulators = AXP806_REG_ID_MAX;
+		axp_cfg.regulators = axp806_regulators;
+		axp_cfg.nregulators = AXP806_REG_ID_MAX;
 
 		/*
 		 * The regulators which are slave in a poly-phase setup
 		 * are skipped, as their controls are bound to the master
 		 * regulator and won't work.
 		 */
-		skip_bitmap |= axp20x_polyphase_slave(axp20x);
+		axp_cfg.skip_bitmap |= axp20x_polyphase_slave(axp20x);
 		break;
 	case AXP809_ID:
-		regulators = axp809_regulators;
-		nregulators = AXP809_REG_ID_MAX;
-		dcdc1_ix = AXP809_DCDC1;
-		dcdc5_ix = AXP809_DCDC5;
-		dc1sw_ix = AXP809_DC1SW;
-		dc5ldo_ix = AXP809_DC5LDO;
+		axp_cfg.regulators = axp809_regulators;
+		axp_cfg.nregulators = AXP809_REG_ID_MAX;
+		axp_cfg.dcdc1_ix = AXP809_DCDC1;
+		axp_cfg.dcdc5_ix = AXP809_DCDC5;
+		axp_cfg.dc1sw_ix = AXP809_DC1SW;
+		axp_cfg.dc5ldo_ix = AXP809_DC5LDO;
 		break;
 	default:
 		dev_err(dev, "Unsupported AXP variant: %ld\n",
@@ -561,86 +281,7 @@ static int axp20x_regulator_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	/* This only sets the dcdc freq. Ignore any errors */
-	axp20x_regulator_parse_dt(dev);
-
-	for (i = 0; i < nregulators; i++) {
-		const struct regulator_desc *desc = &regulators[i];
-		struct regulator_desc *new_desc;
-
-		if (skip_bitmap & (1 << i))
-			continue;
-
-		/*
-		 * Regulators DC1SW and DC5LDO are connected internally,
-		 * so we have to handle their supply names separately.
-		 *
-		 * We always register the regulators in proper sequence,
-		 * so the supply names are correctly read. See the last
-		 * part of this loop to see where we save the DT defined
-		 * name.
-		 */
-		if (i == dc1sw_ix && dcdc1_name) {
-			new_desc = devm_kzalloc(dev, sizeof(*desc),
-						GFP_KERNEL);
-			*new_desc = regulators[i];
-			new_desc->supply_name = dcdc1_name;
-			desc = new_desc;
-		}
-
-		if (i == dc5ldo_ix && dcdc5_name) {
-			new_desc = devm_kzalloc(dev, sizeof(*desc),
-						GFP_KERNEL);
-			*new_desc = regulators[i];
-			new_desc->supply_name = dcdc5_name;
-			desc = new_desc;
-		}
-
-		rdev = devm_regulator_register(dev, desc, &config);
-		if (IS_ERR(rdev)) {
-			dev_err(dev, "Failed to register %s\n",
-				regulators[i].name);
-
-			return PTR_ERR(rdev);
-		}
-
-		ret = of_property_read_u32(rdev->dev.of_node,
-					   "x-powers,dcdc-workmode",
-					   &workmode);
-		if (!ret) {
-			if (axp20x_set_dcdc_workmode(rdev, i, workmode))
-				dev_err(dev, "Failed to set workmode on %s\n",
-					rdev->desc->name);
-		}
-
-		/*
-		 * Save AXP22X DCDC1 / DCDC5 regulator names for later.
-		 */
-		if (i == dcdc1_ix)
-			of_property_read_string(rdev->dev.of_node,
-						"regulator-name",
-						&dcdc1_name);
-
-		if (i == dcdc5_ix)
-			of_property_read_string(rdev->dev.of_node,
-						"regulator-name",
-						&dcdc5_name);
-	}
-
-	if (drivevbus) {
-		/* Change N_VBUSEN sense pin to DRIVEVBUS output pin */
-		regmap_update_bits(axp20x->regmap, AXP20X_OVER_TMP,
-				   AXP22X_MISC_N_VBUSEN_FUNC, 0);
-		rdev = devm_regulator_register(dev,
-					       &axp22x_drivevbus_regulator,
-					       &config);
-		if (IS_ERR(rdev)) {
-			dev_err(dev, "Failed to register drivevbus\n");
-			return PTR_ERR(rdev);
-		}
-	}
-
-	return 0;
+	return axp_regulator_create(dev, &axp_cfg);
 }
 
 static struct platform_driver axp20x_regulator_driver = {
-- 
2.10.0

^ permalink raw reply related

* [PATCH 5/5] arm64: Add uprobe support
From: Catalin Marinas @ 2016-09-22 16:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160922032328.GB29470@localhost.localdomain>

On Thu, Sep 22, 2016 at 08:53:28AM +0530, Pratyush Anand wrote:
> On 21/09/2016:06:04:04 PM, Catalin Marinas wrote:
> > On Wed, Sep 21, 2016 at 04:30:47PM +0530, Pratyush Anand wrote:
> > > On 20/09/2016:05:59:46 PM, Catalin Marinas wrote:
> > > > > +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
> > > > > +		unsigned long addr)
> > > > > +{
> > > > > +	probe_opcode_t insn;
> > > > > +
> > > > > +	/* TODO: Currently we do not support AARCH32 instruction probing */
> > > > 
> > > > Is there a way to check (not necessarily in this file) that we don't
> > > > probe 32-bit tasks?
> > > 
> > > - Well, I do not have complete idea about it that, how it can be done. I think
> > >   we can not check that just by looking a single bit in an instruction.
> > >   My understanding is that, we can only know about it when we are executing the
> > >   instruction, by reading pstate, but that would not be useful for uprobe
> > >   instruction analysis.
> > >   
> > >   I hope, instruction encoding for aarch32 and aarch64 are different, and by
> > >   analyzing for all types of aarch32 instructions, we will be able to decide
> > >   that whether instruction is 32 bit trace-able or not.  Accordingly, we can use
> > >   either BRK or BKPT instruction for breakpoint generation.
> > 
> > We may have some unrelated instruction encoding overlapping but I
> > haven't checked. I was more thinking about whether we know which task is
> > being probed and check is_compat_task() or maybe using
> > compat_user_mode(regs).
> 
> I had thought of this, but problem is that we might not have task in existence
> when we enable uprobes.  For example: Lets say we are inserting a trace probe at
> offset 0x690 in a executable binary.
> 
> echo "p test:0x690" > /sys/kernel/debug/tracing/uprobe_events
> echo 1 > /sys/kernel/debug/tracing/events/uprobes/enable
> 
> In the 'enable' step, it is decided that whether instruction is traceable or
> not. 
> 
> (1) But at this point 'test' executable might not be running.
> (2) Even if it is running, is_compat_task() or compat_user_mode() might not be
> usable, as they work with 'current' task.

What I find strange is that uprobes allows you to insert a breakpoint
instruction that's not even compatible with the task (so it would
SIGILL rather than generate a debug exception).

> What I was thinking that, let it go with 'TODO' as of now. 

Only that I don't have any guarantee that someone is going to fix it ;).

As a quick workaround you could check mm->task_size > TASK_SIZE_32 in
the arch_uprobe_analyze_insn() function.

> Later on, we propose some changes in core layer, so that we can read the elf
> headers of executable binary. ELFCLASS will be able to tell us, whether its a 32
> bit or 64 bit executable. I think, moving "struct uprobe" from
> kernel/events/uprobes.c to a include/linux header file will do the job.  "struct
> arch_uprobe" is part of "struct uprobe". "struct arch_uprobe" is passed in
> arch_uprobe_analyze_insn(). So, we can access struct uprobe's "inode" element
> with this change. 

You can get access to struct linux_binfmt via mm_struct but it doesn't
currently help much since all the members of this structure point to
static functions. Maybe an enum in struct linux_binfmt with format types
exposed to the rest of the kernel?

-- 
Catalin

^ permalink raw reply

* [PATCH] ARM: dts: exynos: Add reboot reason support for Trats2
From: Wolfgang Wiedmeyer @ 2016-09-22 16:48 UTC (permalink / raw)
  To: linux-arm-kernel

This allows to reboot the device into recovery mode and into the download
mode of the bootloader.

Signed-off-by: Wolfgang Wiedmeyer <wolfgit@wiedmeyer.de>
---
 arch/arm/boot/dts/exynos4412-trats2.dts | 14 ++++++++++++++
 arch/arm/boot/dts/exynos4x12.dtsi       |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts b/arch/arm/boot/dts/exynos4412-trats2.dts
index 129e973..a38d1e3 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -1294,3 +1294,17 @@
 	vtmu-supply = <&ldo10_reg>;
 	status = "okay";
 };
+
+&pmu {
+	compatible = "syscon", "simple-mfd";
+
+	reboot-mode {
+		compatible = "syscon-reboot-mode";
+		offset = <0x80c>;
+
+		mode-normal	= <0x12345670>;
+		mode-bootloader	= <0x12345671>;
+		mode-download	= <0x12345671>;
+		mode-recovery	= <0x12345674>;
+	};
+};
diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
index c452499..b58c14e 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -179,7 +179,7 @@
 			ranges;
 			status = "disabled";
 
-			pmu at 10020000 {
+			pmu: pmu at 10020000 {
 				reg = <0x10020000 0x3000>;
 			};
 
-- 
Website: https://fossencdi.org
OpenPGP: 0F30 D1A0 2F73 F70A 6FEE  048E 5816 A24C 1075 7FC4
Key download: https://wiedmeyer.de/keys/ww.asc

^ permalink raw reply related

* [PATCH] arm: dts: fix rk3066a based boards vdd_log voltage initialization
From: Mark Brown @ 2016-09-22 16:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160922171217.09a6d13f@bbrezillon>

On Thu, Sep 22, 2016 at 05:12:17PM +0200, Boris Brezillon wrote:
> +Mark

> I realize Mark has been out of the discussion, and what started as a DT
> problem actually turned into a PWM regulator discussion.
> Maybe we should start a new thread.

Probably, you're lucky I even looked at this - the number of irrelevant
patches I get CCed on is such that I'll often delete things that look
irrelevant unread.  I'm unsure what the relevance is, it looks like it's
mainly a discussion about pinctrl?

> As I said, the problem you're describing (pins muxed to the PWM device
> when it should actually stay in gpio+input mode) is not new, and the old
> pwm-regulator and pwm-rockchip implementation (before my atomic PWM
> changes) were behaving the same way.

Why would this make any kind of sense?

> What is new though, is the pwm_regulator_init_state() function [1], and
> it seems it's now preventing the probe of a pwm-regulator device if the
> initial PWM state is not described in the voltage-table.

> The question is, what should we do?

> 1/ Force users to put an entry matching this state (which means
>    breaking DT compat)
> 2/ Put a valid value in drvdata->state even if it's not reflecting the
>    real state
> 3/ Patch regulator core to support an "unknown-selector" return code.

Could someone say what the actual problem was please?  That was a very
long e-mail so I might be missing something but the obvious thing seems
to be to force a state since we'll be doing that when we enable anyway.
Or just not have the voltage table and use it as a continuous regulator.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160922/bd12da6f/attachment.sig>

^ permalink raw reply

* [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06
From: zhichang.yuan @ 2016-09-22 16:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <9178320.n4yHmfyPA3@wuerfel>


On 09/22/2016 08:14 PM, Arnd Bergmann wrote:
> On Thursday, September 22, 2016 11:55:45 AM CEST Gabriele Paoloni wrote:
>>>> I think extending of_empty_ranges_quirk() may be a reasonable
>>> solution.
>>>> What do you think Arnd?
>>> I don't really like that idea, that quirk is meant to work around
>>> broken DTs, but we can just make the DT valid and implement the
>>> code properly.
>> Ok  I understand your point where it is not right to use of_empty_ranges_quirk()
>> As a quirk is used to work around broken HW or broken FW (as in this case)
>> rather than to fix code
>>
>> What about the following? I think adding the check you suggested next to
>> of_empty_ranges_quirk() is adding the case we need in the right point (thus
>> avoiding any duplication)
>>   
>> --- a/drivers/of/address.c
>> +++ b/drivers/of/address.c
>> @@ -457,6 +457,15 @@ static struct of_bus *of_match_bus(struct device_node *np)
>>          return NULL;
>>   }
>>   
>> +static inline int of_isa_indirect_io(struct device_node *np)
>> +{
>> +       /*
>> +        * check if the current node is an isa bus and if indirectio operation
>> +        * are registered
>> +        */
>> +       return (of_bus_isa_match(np) && arm64_extio_ops);
>> +}
>> +
>>   static int of_empty_ranges_quirk(struct device_node *np)
>>   {
>>          if (IS_ENABLED(CONFIG_PPC)) {
>> @@ -503,7 +512,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>>           * This code is only enabled on powerpc. --gcl
>>           */
>>          ranges = of_get_property(parent, rprop, &rlen);
>> -       if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
>> +       if (ranges == NULL && !of_empty_ranges_quirk(parent) && !of_isa_indirect_io(parent)) {
>>                  pr_debug("OF: no ranges; cannot translate\n");
>>                  return 1;
>>          }
> I don't see what effect that would have. What do you want to
> achieve with this?
>
> I think all we need from this function is to return '1' if
> we hit an ISA I/O window, and that should happen for the two
> interesting cases, either no 'ranges' at all, or no translation
> for the range in question, so that __of_translate_address can
> return OF_BAD_ADDR, and we can enter the special case
> handling in the caller, that handles it like
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 02b2903fe9d2..a18d96843fae 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -685,17 +685,24 @@ static int __of_address_to_resource(struct device_node *dev,
>   	if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
>   		return -EINVAL;
>   	taddr = of_translate_address(dev, addrp);
> -	if (taddr == OF_BAD_ADDR)
> -		return -EINVAL;
>   	memset(r, 0, sizeof(struct resource));
> +
>   	if (flags & IORESOURCE_IO) {
>   		unsigned long port;
> -		port = pci_address_to_pio(taddr);
> +
> +		if (taddr == OF_BAD_ADDR)
> +			port = arch_of_address_to_pio(dev, addrp)
> +		else
> +			port = pci_address_to_pio(taddr);
> +
>   		if (port == (unsigned long)-1)
>   			return -EINVAL;
>   		r->start = port;
>   		r->end = port + size - 1;
>   	} else {
> +		if (taddr == OF_BAD_ADDR)
> +			return -EINVAL;
> +
>   		r->start = taddr;
>   		r->end = taddr + size - 1;
>   	}
>
For this patch sketch, I have a question.
Do we call pci_address_to_pio in arch_of_address_to_pio to get the 
corresponding logical IO port
for LPC??

If we don't, it seems the LPC specific IO address will conflict with PCI 
host bridges' logical IO.
Supposed our LPC populated the IO range from 0x100 to 0x3FF( this is 
normal for ISA similar
devices), after arch_of_address_to_pio(), the r->start will be set as 
0x100, r->end will be set as
0x3FF.  And if there is one PCI host bridge who request a IO window size 
over 0x400 at the same
time, the  corresponding r->start and r->end will be set as 0x0, 0x3FF 
after of_address_to_resource
for this host bridge.  Then the IO conflict happens.

cheers,
Zhichang

>
> 	Arnd

^ permalink raw reply

* [PATCH v26 3/7] arm64: kdump: add kdump support
From: Matthias Brugger @ 2016-09-22 15:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160907042908.6232-4-takahiro.akashi@linaro.org>



On 09/07/2016 06:29 AM, AKASHI Takahiro wrote:
> On crash dump kernel, all the information about primary kernel's system
> memory (core image) is available in elf core header.
> The primary kernel will set aside this header with reserve_elfcorehdr()
> at boot time and inform crash dump kernel of its location via a new
> device-tree property, "linux,elfcorehdr".
>
> Please note that all other architectures use traditional "elfcorehdr="
> kernel parameter for this purpose.
>
> Then crash dump kernel will access the primary kernel's memory with
> copy_oldmem_page(), which reads one page by ioremap'ing it since it does
> not reside in linear mapping on crash dump kernel.
>
> We also need our own elfcorehdr_read() here since the header is placed
> within crash dump kernel's usable memory.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  arch/arm64/Kconfig             | 11 +++++++
>  arch/arm64/kernel/Makefile     |  1 +
>  arch/arm64/kernel/crash_dump.c | 71 ++++++++++++++++++++++++++++++++++++++++++
>  arch/arm64/mm/init.c           | 54 ++++++++++++++++++++++++++++++++
>  4 files changed, 137 insertions(+)
>  create mode 100644 arch/arm64/kernel/crash_dump.c
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index bc3f00f..9c15c66 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -683,6 +683,17 @@ config KEXEC
>  	  but it is independent of the system firmware.   And like a reboot
>  	  you can start any kernel with it, not just Linux.
>
> +config CRASH_DUMP
> +	bool "Build kdump crash kernel"
> +	help
> +	  Generate crash dump after being started by kexec. This should
> +	  be normally only set in special crash dump kernels which are
> +	  loaded in the main kernel with kexec-tools into a specially
> +	  reserved region and then later executed after a crash by
> +	  kdump/kexec.
> +
> +	  For more details see Documentation/kdump/kdump.txt
> +
>  config XEN_DOM0
>  	def_bool y
>  	depends on XEN
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 14f7b65..f1cbfc8 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -48,6 +48,7 @@ arm64-obj-$(CONFIG_RANDOMIZE_BASE)	+= kaslr.o
>  arm64-obj-$(CONFIG_HIBERNATION)		+= hibernate.o hibernate-asm.o
>  arm64-obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o	\
>  					   cpu-reset.o
> +arm64-obj-$(CONFIG_CRASH_DUMP)		+= crash_dump.o
>
>  obj-y					+= $(arm64-obj-y) vdso/ probes/
>  obj-m					+= $(arm64-obj-m)
> diff --git a/arch/arm64/kernel/crash_dump.c b/arch/arm64/kernel/crash_dump.c
> new file mode 100644
> index 0000000..bc5b932
> --- /dev/null
> +++ b/arch/arm64/kernel/crash_dump.c
> @@ -0,0 +1,71 @@
> +/*
> + * Routines for doing kexec-based kdump
> + *
> + * Copyright (C) 2014 Linaro Limited
> + * Author: AKASHI Takahiro <takahiro.akashi@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/crash_dump.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/memblock.h>
> +#include <linux/uaccess.h>
> +#include <asm/memory.h>
> +
> +/**
> + * copy_oldmem_page() - copy one page from old kernel memory
> + * @pfn: page frame number to be copied
> + * @buf: buffer where the copied page is placed
> + * @csize: number of bytes to copy
> + * @offset: offset in bytes into the page
> + * @userbuf: if set, @buf is in a user address space
> + *
> + * This function copies one page from old kernel memory into buffer pointed by
> + * @buf. If @buf is in userspace, set @userbuf to %1. Returns number of bytes
> + * copied or negative error in case of failure.
> + */
> +ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
> +			 size_t csize, unsigned long offset,
> +			 int userbuf)
> +{
> +	void *vaddr;
> +
> +	if (!csize)
> +		return 0;
> +
> +	vaddr = memremap(__pfn_to_phys(pfn), PAGE_SIZE, MEMREMAP_WB);
> +	if (!vaddr)
> +		return -ENOMEM;
> +
> +	if (userbuf) {
> +		if (copy_to_user(buf, vaddr + offset, csize)) {
> +			memunmap(vaddr);
> +			return -EFAULT;
> +		}
> +	} else {
> +		memcpy(buf, vaddr + offset, csize);
> +	}
> +
> +	memunmap(vaddr);
> +
> +	return csize;
> +}
> +
> +/**
> + * elfcorehdr_read - read from ELF core header
> + * @buf: buffer where the data is placed
> + * @csize: number of bytes to read
> + * @ppos: address in the memory
> + *
> + * This function reads @count bytes from elf core header which exists
> + * on crash dump kernel's memory.
> + */
> +ssize_t elfcorehdr_read(char *buf, size_t count, u64 *ppos)
> +{
> +	memcpy(buf, phys_to_virt((phys_addr_t)*ppos), count);
> +	return count;
> +}
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index dd273ec..e4d9c38 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -36,6 +36,7 @@
>  #include <linux/efi.h>
>  #include <linux/swiotlb.h>
>  #include <linux/kexec.h>
> +#include <linux/crash_dump.h>
>
>  #include <asm/boot.h>
>  #include <asm/fixmap.h>
> @@ -186,6 +187,57 @@ static void __init reserve_crashkernel(void)
>  }
>  #endif /* CONFIG_KEXEC_CORE */
>
> +#ifdef CONFIG_CRASH_DUMP
> +static int __init early_init_dt_scan_elfcorehdr(unsigned long node,
> +		const char *uname, int depth, void *data)
> +{
> +	const __be32 *reg;
> +	int len;
> +
> +	if (depth != 1 || strcmp(uname, "chosen") != 0)
> +		return 0;
> +
> +	reg = of_get_flat_dt_prop(node, "linux,elfcorehdr", &len);
> +	if (!reg || (len < (dt_root_addr_cells + dt_root_size_cells)))
> +		return 1;
> +
> +	elfcorehdr_addr = dt_mem_next_cell(dt_root_addr_cells, &reg);
> +	elfcorehdr_size = dt_mem_next_cell(dt_root_size_cells, &reg);
> +
> +	return 1;
> +}
> +
> +/*
> + * reserve_elfcorehdr() - reserves memory for elf core header
> + *
> + * This function reserves elf core header given in "elfcorehdr=" kernel
> + * command line parameter. This region contains all the information about
> + * primary kernel's core image and is used by a dump capture kernel to
> + * access the system memory on primary kernel.
> + */
> +static void __init reserve_elfcorehdr(void)
> +{
> +	of_scan_flat_dt(early_init_dt_scan_elfcorehdr, NULL);
> +

Do I get that right that we can pass crashkernel address/size through 
the kernel boot parameter, but elfcorehdr can only be provided via 
device tree?
Why? If there is a reason for doing so, we should fix the documentation.

Regards,
Matthias

^ permalink raw reply

* [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06
From: zhichang.yuan @ 2016-09-22 15:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F882BB3@lhreml507-mbx>


On 09/22/2016 11:20 PM, Gabriele Paoloni wrote:
>
>> -----Original Message-----
>> From: Arnd Bergmann [mailto:arnd at arndb.de]
>> Sent: 22 September 2016 15:59
>> To: Gabriele Paoloni
>> Cc: zhichang; linux-arm-kernel at lists.infradead.org;
>> devicetree at vger.kernel.org; lorenzo.pieralisi at arm.com; minyard at acm.org;
>> linux-pci at vger.kernel.org; gregkh at linuxfoundation.org; John Garry;
>> will.deacon at arm.com; linux-kernel at vger.kernel.org; Yuanzhichang;
>> Linuxarm; xuwei (O); linux-serial at vger.kernel.org;
>> benh at kernel.crashing.org; zourongrong at gmail.com; liviu.dudau at arm.com;
>> kantyzc at 163.com
>> Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on
>> Hip06
>>
>> On Thursday, September 22, 2016 2:47:14 PM CEST Gabriele Paoloni wrote:
>>>>>   static int of_empty_ranges_quirk(struct device_node *np)
>>>>>   {
>>>>>          if (IS_ENABLED(CONFIG_PPC)) {
>>>>> @@ -503,7 +512,7 @@ static int of_translate_one(struct
>> device_node
>>>> *parent, struct of_bus *bus,
>>>>>           * This code is only enabled on powerpc. --gcl
>>>>>           */
>>>>>          ranges = of_get_property(parent, rprop, &rlen);
>>>>> -       if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
>>>>> +       if (ranges == NULL && !of_empty_ranges_quirk(parent) &&
>>>> !of_isa_indirect_io(parent)) {
>>>>>                  pr_debug("OF: no ranges; cannot translate\n");
>>>>>                  return 1;
>>>>>          }
>>>> I don't see what effect that would have. What do you want to
>>>> achieve with this?
>>> If I read the code correctly adding the function above would end
>>> up in a 1:1 mapping:
>>> http://lxr.free-electrons.com/source/drivers/of/address.c#L513
>>>
>>> so taddr will be assigned with the cpu address space specified
>>> in the children nodes of LPC and we are not using a quirk function
>>> (we are just checking that we have the indirect io assigned and
>>> that we are on a ISA bus). Now probably there is a nit in my
>>> code sketch where of_isa_indirect_io should be probably an
>> architecture
>>> specific function...
>> But the point is that it would then return an incorrect address,
>> which in the worst case could be the same as another I/O space
>> if that happens to be at CPU address zero.
> If we do not touch __of_address_to_resource after taddr is returned
> by of_translate_address we will check for (flags & IORESOURCE_IO),
> then we call pci_address_to_pio to retrieve the unique token (remember
> that LPC driver will register the LPC io range to pci io_range_list).
>
> I do not think that we can have any conflict with any other I/O space
> as pci_register_io_range will guarantee that the LPC range does not
> overlap with any other I/O range...
If we don't bypass the calling of pci_address_to_pio after 
of_translate_address,
there should no conflict between LPC logical IO range and other logical 
IO ranges
of other devices.
I guess Arnd want to skip all the translation for our LPC IO address. 
But if we do it
like that, it seems we can't avoid the possible conflict with the 
logical IO ranges of
PCI host bridges without any changes on the pci_register_io_range and 
pci_address_to_pio.
Because two completely separate I/O spaces are created without 
synchronization.

Best,
Zhichang
>>>> I think all we need from this function is to return '1' if
>>>> we hit an ISA I/O window, and that should happen for the two
>>>> interesting cases, either no 'ranges' at all, or no translation
>>>> for the range in question, so that __of_translate_address can
>>>> return OF_BAD_ADDR, and we can enter the special case
>>>> handling in the caller, that handles it like
>>>>
>>> I don't think this is very right as you may fail for different
>>> reasons other than a missing range property, e.g:
>>> http://lxr.free-electrons.com/source/drivers/of/address.c#L575
>>>
>>> And even if the only failure case was a missing range if in the
>>> future __of_translate_address had to be reworked we would again
>>> make a wrong assumption...you get my point?
>> The newly introduced function would clearly have to make
>> some sanity checks. The idea is that treat the case of
>> not being able to translate a bus specific I/O address
>> into a CPU address literally and fall back to another method
>> of translating that address.
>>
>> This matches my mental model of how we find the resource:
>>
>> - start with the bus address
>> - try to translate that into a CPU address
>> - if we arrive at a CPU physical address for IORESOURCE_MEM, use that
>> - if we arrive at a CPU physical address for IORESOURCE_IO, translate
>>    that into a Linux IORESOURCE_IO token
>> - if there is no valid CPU physical address, try to translate
>>    the address into an IORESOURCE_IO using the ISA accessor
>> - if that fails too, give up.
>>
>> If you try to fake a CPU physical address inbetween, it just
>> gets more confusing.
>>
>> 	Arnd

^ permalink raw reply

* [GIT PULL] iommu/arm-smmu: Updates for 4.9
From: Joerg Roedel @ 2016-09-22 15:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160920102534.GC17978@arm.com>

On Tue, Sep 20, 2016 at 11:25:34AM +0100, Will Deacon wrote:
> Hi Joerg,
> 
> Please pull these significant arm-smmu updates for 4.9. The vast majority
> of the code here is Robin's series to move the drivers over to the
> generic DT bindings, which finally hooks up the DMA API and MSI mapping
> for host machines. This has been stewing for a while and is now at the
> point where I feel it's much better in mainline than languishing outside.
> 
> A consequence of that is that there are some non-trivial changes outside
> of the drivers, but these are all acked by the relevant subsystem
> maintainers.
> 
> Other stuff here includes some non-critical fixes to the queue handling
> on SMMUv3 and a small devm cleanup that missed the boat last time around.
> 
> Cheers,
> 
> Will
> 
> --->8
> 
> The following changes since commit 3eab887a55424fc2c27553b7bfe32330df83f7b8:
> 
>   Linux 4.8-rc4 (2016-08-28 15:04:33 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git for-joerg/arm-smmu/updates

Pulled, thanks Will.

^ permalink raw reply

* [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06
From: Gabriele Paoloni @ 2016-09-22 15:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1669038.JevuM4F83e@wuerfel>



> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 22 September 2016 15:59
> To: Gabriele Paoloni
> Cc: zhichang; linux-arm-kernel at lists.infradead.org;
> devicetree at vger.kernel.org; lorenzo.pieralisi at arm.com; minyard at acm.org;
> linux-pci at vger.kernel.org; gregkh at linuxfoundation.org; John Garry;
> will.deacon at arm.com; linux-kernel at vger.kernel.org; Yuanzhichang;
> Linuxarm; xuwei (O); linux-serial at vger.kernel.org;
> benh at kernel.crashing.org; zourongrong at gmail.com; liviu.dudau at arm.com;
> kantyzc at 163.com
> Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on
> Hip06
> 
> On Thursday, September 22, 2016 2:47:14 PM CEST Gabriele Paoloni wrote:
> > > >  static int of_empty_ranges_quirk(struct device_node *np)
> > > >  {
> > > >         if (IS_ENABLED(CONFIG_PPC)) {
> > > > @@ -503,7 +512,7 @@ static int of_translate_one(struct
> device_node
> > > *parent, struct of_bus *bus,
> > > >          * This code is only enabled on powerpc. --gcl
> > > >          */
> > > >         ranges = of_get_property(parent, rprop, &rlen);
> > > > -       if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
> > > > +       if (ranges == NULL && !of_empty_ranges_quirk(parent) &&
> > > !of_isa_indirect_io(parent)) {
> > > >                 pr_debug("OF: no ranges; cannot translate\n");
> > > >                 return 1;
> > > >         }
> > >
> > > I don't see what effect that would have. What do you want to
> > > achieve with this?
> >
> > If I read the code correctly adding the function above would end
> > up in a 1:1 mapping:
> > http://lxr.free-electrons.com/source/drivers/of/address.c#L513
> >
> > so taddr will be assigned with the cpu address space specified
> > in the children nodes of LPC and we are not using a quirk function
> > (we are just checking that we have the indirect io assigned and
> > that we are on a ISA bus). Now probably there is a nit in my
> > code sketch where of_isa_indirect_io should be probably an
> architecture
> > specific function...
> 
> But the point is that it would then return an incorrect address,
> which in the worst case could be the same as another I/O space
> if that happens to be at CPU address zero.

If we do not touch __of_address_to_resource after taddr is returned
by of_translate_address we will check for (flags & IORESOURCE_IO),
then we call pci_address_to_pio to retrieve the unique token (remember
that LPC driver will register the LPC io range to pci io_range_list).

I do not think that we can have any conflict with any other I/O space
as pci_register_io_range will guarantee that the LPC range does not
overlap with any other I/O range... 

> 
> > > I think all we need from this function is to return '1' if
> > > we hit an ISA I/O window, and that should happen for the two
> > > interesting cases, either no 'ranges' at all, or no translation
> > > for the range in question, so that __of_translate_address can
> > > return OF_BAD_ADDR, and we can enter the special case
> > > handling in the caller, that handles it like
> > >
> >
> > I don't think this is very right as you may fail for different
> > reasons other than a missing range property, e.g:
> > http://lxr.free-electrons.com/source/drivers/of/address.c#L575
> >
> > And even if the only failure case was a missing range if in the
> > future __of_translate_address had to be reworked we would again
> > make a wrong assumption...you get my point?
> 
> The newly introduced function would clearly have to make
> some sanity checks. The idea is that treat the case of
> not being able to translate a bus specific I/O address
> into a CPU address literally and fall back to another method
> of translating that address.
> 
> This matches my mental model of how we find the resource:
> 
> - start with the bus address
> - try to translate that into a CPU address
> - if we arrive at a CPU physical address for IORESOURCE_MEM, use that
> - if we arrive at a CPU physical address for IORESOURCE_IO, translate
>   that into a Linux IORESOURCE_IO token
> - if there is no valid CPU physical address, try to translate
>   the address into an IORESOURCE_IO using the ISA accessor
> - if that fails too, give up.
> 
> If you try to fake a CPU physical address inbetween, it just
> gets more confusing.
> 
> 	Arnd

^ permalink raw reply

* [PATCH] arm: dts: fix rk3066a based boards vdd_log voltage initialization
From: Boris Brezillon @ 2016-09-22 15:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAD=FV=W=ZiaLu1=zcnDyUqvK6f=E-0AWH9URHE3cgAKWWa+aJA@mail.gmail.com>

+Mark

I realize Mark has been out of the discussion, and what started as a DT
problem actually turned into a PWM regulator discussion.
Maybe we should start a new thread.

On Mon, 19 Sep 2016 14:15:06 -0700
Doug Anderson <dianders@chromium.org> wrote:

> Hi,
> 
> On Mon, Sep 19, 2016 at 1:43 PM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Mon, 19 Sep 2016 11:12:12 -0700
> > Doug Anderson <dianders@chromium.org> wrote:
> >  
> >> Hi,
> >>
> >> On Mon, Sep 19, 2016 at 11:06 AM, Boris Brezillon
> >> <boris.brezillon@free-electrons.com> wrote:  
> >> > On Mon, 19 Sep 2016 10:52:51 -0700
> >> > Doug Anderson <dianders@chromium.org> wrote:
> >> >  
> >> >> Hi,
> >> >>
> >> >> On Mon, Sep 19, 2016 at 10:48 AM, Boris Brezillon
> >> >> <boris.brezillon@free-electrons.com> wrote:  
> >> >> > The PWM chip has always claimed the pins and muxed them to the PWM IP.
> >> >> > So, this means it's broken from the beginning, and my patch is only
> >> >> > uncovering the problem (unless the pins stay configured as input until
> >> >> > the PWM is enabled, which I'm not sure is the case).  
> >> >>
> >> >> Such a solution is achievable with the pinctrl APIs pretty easily.
> >> >> You might not be able to use the automatic "init" state but you can do
> >> >> something similar and switch to an "active" state once the PWM is
> >> >> actually turned on the first time.  
> >> >
> >> > But is it really the case here (I don't see any code requesting a
> >> > specific pinmux depending on the PWM state)?  
> >>
> >> It is not happening right now as far as I know.  ...but that's a bug.
> >>  
> >> > Anyway, we really need to handle this case, we should define the
> >> > typical voltage when the PWM is disabled. Same as what you suggested
> >> > with voltage-when-input, but with a different naming (since the concept
> >> > of pinmux is PWM hardware/driver specific).
> >> >
> >> >         voltage-when-pwm-disabled = <...>;  
> >>
> >> Voltage when disabled and voltage when input are two different states.
> >> A disabled PWM will typically either drive high or low (depending on
> >> where it was when you turned it off).  Not all "disabled" states will
> >> mean that the pin is configured as an input.  
> >
> > Okay, after reading again your first answer, I think I understand why
> > you want to differentiate the when-disabled and when-input cases. You
> > want to use the "init" and "default" pinctrl states. The "init" state
> > (applied at probe time) would keep the PWM pin in gpio+input mode and
> > the "default" state (applied when the PWM device is enabled for the
> > first time) would mux the pin to the PWM device.
> > Your solution requires that the pwm-regulator device knows in which
> > state the pin attached to the PWM is, which IMO is breaking the
> > layering we have right now: a PWM-regulator is assigned a PWM device
> > which is assigned a pin and a pinmux config.
> > Another solution would be to expose an additional information in the
> > pwm_state: whether the PWM is in the INIT state (probed, but not yet
> > configured by its user) or DEFAULT state (probed and already
> > configured by its user). But again, by doing that we also expose
> > internal PWM details to its user, which I'm not sure will help keep
> > the PWM API simple.  
> 
> One note is that probably using the "init" state would not be a good
> idea because it would make assumptions about what state the firmware
> left things.  Possibly a firmware update could cause a change from a
> PWM being left as "input" to the PWM actually being initted.
> ...really we should be able to detect if the PWM was left on at
> bootup.

Yep.

> 
> 
> > Actually, I had something slightly different in mind. I thought about
> > having two new pinctrl states ("enabled" and "disabled"). The "enabled"
> > state (pin muxed to the PWM device) would be applied each time the PWM
> > is enabled, and "disabled" state (gpio+input mode) would be applied each
> > time the PWM is disabled.  
> 
> Adding "enabled" and "disabled" state is sane IMHO.  At the moment the
> PWM regulator never actually puts things in "disabled" state, but I
> suppose it could in the future.
> 
> > This way we can guarantee that even when the PWM is disabled, the
> > PWM-driven regulator is configured to output a non-destructive voltage.
> > Hence my suggestion to name the property 'voltage-when-pwm-disabled'.  
> 
> That's fine with me, as long as the PWM starts up as "enabled" if the
> pinctrl happened to be left initted by the BIOS.

Yes, that's already the case in the pwm-rockchip driver.

Okay, I can try to implement what is described above, but, in the
meantime, I think we should find a solution for Andy's initial problem.

As I said, the problem you're describing (pins muxed to the PWM device
when it should actually stay in gpio+input mode) is not new, and the old
pwm-regulator and pwm-rockchip implementation (before my atomic PWM
changes) were behaving the same way.

What is new though, is the pwm_regulator_init_state() function [1], and
it seems it's now preventing the probe of a pwm-regulator device if the
initial PWM state is not described in the voltage-table.

The question is, what should we do?

1/ Force users to put an entry matching this state (which means
   breaking DT compat)
2/ Put a valid value in drvdata->state even if it's not reflecting the
   real state
3/ Patch regulator core to support an "unknown-selector" return code.

Mark, any opinion?

Thanks,

Boris

[1]https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/regulator/pwm-regulator.c?id=refs/tags/next-20160922#n60

^ permalink raw reply

* [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-09-22 14:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <EE11001F9E5DDD47B7634E2F8A612F2E1F882AD3@lhreml507-mbx>

On Thursday, September 22, 2016 2:47:14 PM CEST Gabriele Paoloni wrote:
> > >  static int of_empty_ranges_quirk(struct device_node *np)
> > >  {
> > >         if (IS_ENABLED(CONFIG_PPC)) {
> > > @@ -503,7 +512,7 @@ static int of_translate_one(struct device_node
> > *parent, struct of_bus *bus,
> > >          * This code is only enabled on powerpc. --gcl
> > >          */
> > >         ranges = of_get_property(parent, rprop, &rlen);
> > > -       if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
> > > +       if (ranges == NULL && !of_empty_ranges_quirk(parent) &&
> > !of_isa_indirect_io(parent)) {
> > >                 pr_debug("OF: no ranges; cannot translate\n");
> > >                 return 1;
> > >         }
> > 
> > I don't see what effect that would have. What do you want to
> > achieve with this?
> 
> If I read the code correctly adding the function above would end
> up in a 1:1 mapping:
> http://lxr.free-electrons.com/source/drivers/of/address.c#L513
> 
> so taddr will be assigned with the cpu address space specified
> in the children nodes of LPC and we are not using a quirk function
> (we are just checking that we have the indirect io assigned and
> that we are on a ISA bus). Now probably there is a nit in my 
> code sketch where of_isa_indirect_io should be probably an architecture
> specific function...

But the point is that it would then return an incorrect address,
which in the worst case could be the same as another I/O space
if that happens to be at CPU address zero.

> > I think all we need from this function is to return '1' if
> > we hit an ISA I/O window, and that should happen for the two
> > interesting cases, either no 'ranges' at all, or no translation
> > for the range in question, so that __of_translate_address can
> > return OF_BAD_ADDR, and we can enter the special case
> > handling in the caller, that handles it like
> > 
> 
> I don't think this is very right as you may fail for different
> reasons other than a missing range property, e.g:
> http://lxr.free-electrons.com/source/drivers/of/address.c#L575
> 
> And even if the only failure case was a missing range if in the
> future __of_translate_address had to be reworked we would again
> make a wrong assumption...you get my point?

The newly introduced function would clearly have to make
some sanity checks. The idea is that treat the case of
not being able to translate a bus specific I/O address
into a CPU address literally and fall back to another method
of translating that address.

This matches my mental model of how we find the resource:

- start with the bus address
- try to translate that into a CPU address
- if we arrive at a CPU physical address for IORESOURCE_MEM, use that
- if we arrive at a CPU physical address for IORESOURCE_IO, translate
  that into a Linux IORESOURCE_IO token
- if there is no valid CPU physical address, try to translate
  the address into an IORESOURCE_IO using the ISA accessor
- if that fails too, give up.

If you try to fake a CPU physical address inbetween, it just
gets more confusing.

	Arnd

^ permalink raw reply

* [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on Hip06
From: Gabriele Paoloni @ 2016-09-22 14:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <9178320.n4yHmfyPA3@wuerfel>

Hi Arnd

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 22 September 2016 13:15
> To: Gabriele Paoloni
> Cc: zhichang; linux-arm-kernel at lists.infradead.org;
> devicetree at vger.kernel.org; lorenzo.pieralisi at arm.com; minyard at acm.org;
> linux-pci at vger.kernel.org; gregkh at linuxfoundation.org; John Garry;
> will.deacon at arm.com; linux-kernel at vger.kernel.org; Yuanzhichang;
> Linuxarm; xuwei (O); linux-serial at vger.kernel.org;
> benh at kernel.crashing.org; zourongrong at gmail.com; liviu.dudau at arm.com;
> kantyzc at 163.com
> Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on
> Hip06
> 
> On Thursday, September 22, 2016 11:55:45 AM CEST Gabriele Paoloni
> wrote:
> > > > I think extending of_empty_ranges_quirk() may be a reasonable
> > > solution.
> > > > What do you think Arnd?
> > >
> > > I don't really like that idea, that quirk is meant to work around
> > > broken DTs, but we can just make the DT valid and implement the
> > > code properly.
> >
> > Ok  I understand your point where it is not right to use
> of_empty_ranges_quirk()
> > As a quirk is used to work around broken HW or broken FW (as in this
> case)
> > rather than to fix code
> >
> > What about the following? I think adding the check you suggested next
> to
> > of_empty_ranges_quirk() is adding the case we need in the right point
> (thus
> > avoiding any duplication)
> >
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -457,6 +457,15 @@ static struct of_bus *of_match_bus(struct
> device_node *np)
> >         return NULL;
> >  }
> >
> > +static inline int of_isa_indirect_io(struct device_node *np)
> > +{
> > +       /*
> > +        * check if the current node is an isa bus and if indirectio
> operation
> > +        * are registered
> > +        */
> > +       return (of_bus_isa_match(np) && arm64_extio_ops);
> > +}
> > +
> >  static int of_empty_ranges_quirk(struct device_node *np)
> >  {
> >         if (IS_ENABLED(CONFIG_PPC)) {
> > @@ -503,7 +512,7 @@ static int of_translate_one(struct device_node
> *parent, struct of_bus *bus,
> >          * This code is only enabled on powerpc. --gcl
> >          */
> >         ranges = of_get_property(parent, rprop, &rlen);
> > -       if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
> > +       if (ranges == NULL && !of_empty_ranges_quirk(parent) &&
> !of_isa_indirect_io(parent)) {
> >                 pr_debug("OF: no ranges; cannot translate\n");
> >                 return 1;
> >         }
> 
> I don't see what effect that would have. What do you want to
> achieve with this?

If I read the code correctly adding the function above would end
up in a 1:1 mapping:
http://lxr.free-electrons.com/source/drivers/of/address.c#L513

so taddr will be assigned with the cpu address space specified
in the children nodes of LPC and we are not using a quirk function
(we are just checking that we have the indirect io assigned and
that we are on a ISA bus). Now probably there is a nit in my 
code sketch where of_isa_indirect_io should be probably an architecture
specific function...

> 
> I think all we need from this function is to return '1' if
> we hit an ISA I/O window, and that should happen for the two
> interesting cases, either no 'ranges' at all, or no translation
> for the range in question, so that __of_translate_address can
> return OF_BAD_ADDR, and we can enter the special case
> handling in the caller, that handles it like
> 

I don't think this is very right as you may fail for different
reasons other than a missing range property, e.g:
http://lxr.free-electrons.com/source/drivers/of/address.c#L575

And even if the only failure case was a missing range if in the
future __of_translate_address had to be reworked we would again
make a wrong assumption...you get my point?

Thanks

Gab

> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 02b2903fe9d2..a18d96843fae 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -685,17 +685,24 @@ static int __of_address_to_resource(struct
> device_node *dev,
>  	if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
>  		return -EINVAL;
>  	taddr = of_translate_address(dev, addrp);
> -	if (taddr == OF_BAD_ADDR)
> -		return -EINVAL;
>  	memset(r, 0, sizeof(struct resource));
> +
>  	if (flags & IORESOURCE_IO) {
>  		unsigned long port;
> -		port = pci_address_to_pio(taddr);
> +
> +		if (taddr == OF_BAD_ADDR)
> +			port = arch_of_address_to_pio(dev, addrp)
> +		else
> +			port = pci_address_to_pio(taddr);
> +
>  		if (port == (unsigned long)-1)
>  			return -EINVAL;
>  		r->start = port;
>  		r->end = port + size - 1;
>  	} else {
> +		if (taddr == OF_BAD_ADDR)
> +			return -EINVAL;
> +
>  		r->start = taddr;
>  		r->end = taddr + size - 1;
>  	}
> 
> 
> 
> 	Arnd

^ permalink raw reply

* [PATCH] clocksource: timer-digicolor: Unmap region obtained by of_iomap
From: Baruch Siach @ 2016-09-22 14:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474554073-8658-1-git-send-email-arvind.yadav.cs@gmail.com>

Hi Arvind Yadav,

On Thu, Sep 22, 2016 at 07:51:13PM +0530, Arvind Yadav wrote:
> Free memory mapping, if digicolor_timer_init is not successful.

Thanks for looking into that.

Why not add a devm_of_iomap() variant of of_iomap() that calls devm_ioremap() 
instead of ioremap()?

I'm not sure that cleaning up after a clocksource driver init failure makes 
much sense. Something is clearly fundamentally wrong in your device tree, and 
your system will most likely not boot anyway.

baruch

> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
>  drivers/clocksource/timer-digicolor.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/clocksource/timer-digicolor.c b/drivers/clocksource/timer-digicolor.c
> index 10318cc..e6e5e6a 100644
> --- a/drivers/clocksource/timer-digicolor.c
> +++ b/drivers/clocksource/timer-digicolor.c
> @@ -167,12 +167,14 @@ static int __init digicolor_timer_init(struct device_node *node)
>  	irq = irq_of_parse_and_map(node, dc_timer_dev.timer_id);
>  	if (irq <= 0) {
>  		pr_err("Can't parse IRQ");
> +		iounmap(dc_timer_dev.base);
>  		return -EINVAL;
>  	}
>  
>  	clk = of_clk_get(node, 0);
>  	if (IS_ERR(clk)) {
>  		pr_err("Can't get timer clock");
> +		iounmap(dc_timer_dev.base);
>  		return PTR_ERR(clk);
>  	}
>  	clk_prepare_enable(clk);
> @@ -192,6 +194,7 @@ static int __init digicolor_timer_init(struct device_node *node)
>  			  &dc_timer_dev.ce);
>  	if (ret) {
>  		pr_warn("request of timer irq %d failed (%d)\n", irq, ret);
> +		iounmap(dc_timer_dev.base);
>  		return ret;
>  	}

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

^ permalink raw reply

* [PATCH v2 2/3] musb: sunxi: Remove custom babble handling
From: Bin Liu @ 2016-09-22 14:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <39199bd7-a372-0312-6177-5b0fc2d683ac@redhat.com>

Hi,

On Thu, Sep 22, 2016 at 05:03:39PM +0300, Hans de Goede wrote:
> Hi,
> 
> On 09/22/2016 04:54 PM, Bin Liu wrote:
> >Hi,
> >
> >On Thu, Sep 22, 2016 at 02:19:00PM +0300, Hans de Goede wrote:
> >>The musb-core now a days always treats babble errors in host mode
> >
> >I don't think this statement is accurate. You might want to change it to
> >"The musb core already handles babble interrupt" or something else.
> 
> It is accurate if you look in the history at drivers/usb/musb
> commits around 15-03-10 you will see 2 relevant commits:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/musb?id=b4dc38fd45b63e3da2bc98db5d283a15a637a2fa
> 
> "usb: musb: core: simplify musb_recover_work()"
> 
> This commits introduces calling musb_root_disconnect(musb)
> on babble errors, that was not happening before which is why

That is true, but calling musb_root_disconnect() is just one step of the
recovery in musb core, not all.

The statement of "treats babble errors in host mode as disconnects"
implies all the babble handling is just disconnect, which is not
accurate.

BTY, "babble errors in host mode" is also redundant. babble implies
host mode.

Regards,
-Bin.
> I added the custom babble error handling. to the sunxi glue.
> 
> And:
> 
> "usb: musb: core: always try to recover from babble"
> 
> Where the title says it all.
> 
> Take these together and I believe that my commit msg:
> 
> "The musb-core now a days always treats babble errors in host mode
> as disconnects, so there is no need for the sunxi specific handling
> of this anymore."
> 
> Is quite accurate.
> 
> Regards,
> 
> Hans
> 
> 
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>---
> >>Changes in v2:
> >>-This is a new patch in v2 of this patch series
> >>---
> >> drivers/usb/musb/sunxi.c | 10 ----------
> >> 1 file changed, 10 deletions(-)
> >>
> >>diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> >>index 1408245..82eba92 100644
> >>--- a/drivers/usb/musb/sunxi.c
> >>+++ b/drivers/usb/musb/sunxi.c
> >>@@ -186,16 +186,6 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
> >> 	if (musb->int_usb)
> >> 		writeb(musb->int_usb, musb->mregs + SUNXI_MUSB_INTRUSB);
> >>
> >>-	/*
> >>-	 * sunxi musb often signals babble on low / full speed device
> >>-	 * disconnect, without ever raising MUSB_INTR_DISCONNECT, since
> >>-	 * normally babble never happens treat it as disconnect.
> >>-	 */
> >>-	if ((musb->int_usb & MUSB_INTR_BABBLE) && is_host_active(musb)) {
> >>-		musb->int_usb &= ~MUSB_INTR_BABBLE;
> >>-		musb->int_usb |= MUSB_INTR_DISCONNECT;
> >>-	}
> >>-
> >> 	if ((musb->int_usb & MUSB_INTR_RESET) && !is_host_active(musb)) {
> >> 		/* ep0 FADDR must be 0 when (re)entering peripheral mode */
> >> 		musb_ep_select(musb->mregs, 0);
> >>--
> >>2.9.3
> >>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] clocksource/drivers/ti-32k: Prevent ftrace recursion
From: Steven Rostedt @ 2016-09-22 14:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.20.1609221555590.5599@nanos>

On Thu, 22 Sep 2016 15:58:03 +0200 (CEST)
Thomas Gleixner <tglx@linutronix.de> wrote:

> On Thu, 22 Sep 2016, Jisheng Zhang wrote:
> 
> > Currently ti-32k can be used as a scheduler clock. We properly marked
> > omap_32k_read_sched_clock() as notrace but we then call another
> > function ti_32k_read_cycles() that _wasn't_ notrace.
> > 
> > Having a traceable function in the sched_clock() path leads to a
> > recursion within ftrace and a kernel crash.  
> 
> Kernel crash? Doesn't ftrace core prevent recursion?
>

There is recursion protection, but there are some holes, as well as
calls where ftrace can't protect itself.

What triggered the bug? Just simple enabling of function tracing? And
what arch? I would like to close these holes. Although, I should add
some kind of flag to notify the user (or at least for me) that recursion
is happening, because that can really be a performance hit on tracing.

-- Steve

^ permalink raw reply

* [PATCH] clocksource: timer-digicolor: Unmap region obtained by of_iomap
From: Arvind Yadav @ 2016-09-22 14:21 UTC (permalink / raw)
  To: linux-arm-kernel

Free memory mapping, if digicolor_timer_init is not successful.

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/clocksource/timer-digicolor.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clocksource/timer-digicolor.c b/drivers/clocksource/timer-digicolor.c
index 10318cc..e6e5e6a 100644
--- a/drivers/clocksource/timer-digicolor.c
+++ b/drivers/clocksource/timer-digicolor.c
@@ -167,12 +167,14 @@ static int __init digicolor_timer_init(struct device_node *node)
 	irq = irq_of_parse_and_map(node, dc_timer_dev.timer_id);
 	if (irq <= 0) {
 		pr_err("Can't parse IRQ");
+		iounmap(dc_timer_dev.base);
 		return -EINVAL;
 	}
 
 	clk = of_clk_get(node, 0);
 	if (IS_ERR(clk)) {
 		pr_err("Can't get timer clock");
+		iounmap(dc_timer_dev.base);
 		return PTR_ERR(clk);
 	}
 	clk_prepare_enable(clk);
@@ -192,6 +194,7 @@ static int __init digicolor_timer_init(struct device_node *node)
 			  &dc_timer_dev.ce);
 	if (ret) {
 		pr_warn("request of timer irq %d failed (%d)\n", irq, ret);
+		iounmap(dc_timer_dev.base);
 		return ret;
 	}
 
-- 
2.7.4

^ permalink raw reply related


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