Devicetree
 help / color / mirror / Atom feed
* Re: [RFC PATCH 3/3] of: fix node traversing in of_dma_get_range
From: Robin Murphy @ 2017-03-27 14:45 UTC (permalink / raw)
  To: Rob Herring, Oza Pawandeep
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux IOMMU,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CAL_Jsq+1f2pHEy7=2Q7xGYYJjuQk3rLHH4pPTKRVN4QTtSr4Kw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Rob,

On 27/03/17 15:34, Rob Herring wrote:
> On Sat, Mar 25, 2017 at 12:31 AM, Oza Pawandeep <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>> it jumps to the parent node without examining the child node.
>> also with that, it throws "no dma-ranges found for node"
>> for pci dma-ranges.
>>
>> this patch fixes device node traversing for dma-ranges.
> 
> What's the DT look like that doesn't work?

The problem is the bodge in pci_dma_configure() where we don't have an
OF node for the actual device itself, so pass in the host bridge's OF
node instead. This happens to work well enough for dma-coherent, but I
don't think dma-ranges was even considered at the time.

As it happens I'm currently halfway through writing an experiment
wherein pci_dma_configure() creates a temporary child node for the
of_dma_configure() call if no other suitable alternative (e.g. some
intermediate bridge node) exists. How hard are you likely to NAK that
approach? ;)

> dma-ranges is supposed to be a bus property, not a device's property.
> So looking at the parent is correct behavior generally.

Indeed, this patch as-is will break currently correct DTs (because we
won't find dma-ranges on the device, so will bail before even looking at
the parent as we should).

Robin.

> 
> Rob
> 

^ permalink raw reply

* Re: [RFC PATCH 1/3] of/pci: dma-ranges to account highest possible host bridge dma_mask
From: Rob Herring @ 2017-03-27 14:46 UTC (permalink / raw)
  To: Oza Pawandeep
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux IOMMU,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <1490419893-5073-1-git-send-email-oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On Sat, Mar 25, 2017 at 12:31 AM, Oza Pawandeep <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> it is possible that PCI device supports 64-bit DMA addressing,
> and thus it's driver sets device's dma_mask to DMA_BIT_MASK(64),
> however PCI host bridge may have limitations on the inbound
> transaction addressing. As an example, consider NVME SSD device
> connected to iproc-PCIe controller.
>
> Currently, the IOMMU DMA ops only considers PCI device dma_mask
> when allocating an IOVA. This is particularly problematic on
> ARM/ARM64 SOCs where the IOMMU (i.e. SMMU) translates IOVA to
> PA for in-bound transactions only after PCI Host has forwarded
> these transactions on SOC IO bus. This means on such ARM/ARM64
> SOCs the IOVA of in-bound transactions has to honor the addressing
> restrictions of the PCI Host.
>
> current pcie frmework and of framework integration assumes dma-ranges
> in a way where memory-mapped devices define their dma-ranges.
> dma-ranges: (child-bus-address, parent-bus-address, length).
>
> but iproc based SOCs and even Rcar based SOCs has PCI world dma-ranges.
> dma-ranges = <0x43000000 0x00 0x00 0x00 0x00 0x80 0x00>;

If you implement a common function, then I expect to see other users
converted to use it. There's also PCI hosts in arch/powerpc that parse
dma-ranges.

Rob

^ permalink raw reply

* Re: [PATCH V6 4/7] mfd: da9061: MFD core support
From: kbuild test robot @ 2017-03-27 14:51 UTC (permalink / raw)
  To: Steve Twiss
  Cc: kbuild-all, LINUX-KERNEL, Lee Jones, DEVICETREE, Dmitry Torokhov,
	Eduardo Valentin, Guenter Roeck, LINUX-INPUT, LINUX-PM,
	LINUX-WATCHDOG, Liam Girdwood, Mark Brown, Mark Rutland,
	Rob Herring, Support Opensource, Wim Van Sebroeck, Zhang Rui
In-Reply-To: <539b8756120ab3558c906f7457e65f94dbe37425.1490608293.git.stwiss.opensource@diasemi.com>

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

Hi Steve,

[auto build test WARNING on ljones-mfd/for-mfd-next]
[also build test WARNING on v4.11-rc4 next-20170327]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Steve-Twiss/Documentation-devicetree-watchdog-da9062-61-watchdog-timer-binding/20170327-215359
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: x86_64-randconfig-x009-201713 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers//mfd/da9062-core.c: In function 'da9062_i2c_probe':
>> drivers//mfd/da9062-core.c:845:21: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
      chip->chip_type = (int)match->data;
                        ^

vim +845 drivers//mfd/da9062-core.c

   829		unsigned int irq_base;
   830		const struct mfd_cell *cell;
   831		const struct regmap_irq_chip *irq_chip;
   832		const struct regmap_config *config;
   833		int cell_num;
   834		int ret;
   835	
   836		chip = devm_kzalloc(&i2c->dev, sizeof(*chip), GFP_KERNEL);
   837		if (!chip)
   838			return -ENOMEM;
   839	
   840		if (i2c->dev.of_node) {
   841			match = of_match_node(da9062_dt_ids, i2c->dev.of_node);
   842			if (!match)
   843				return -EINVAL;
   844	
 > 845			chip->chip_type = (int)match->data;
   846		} else {
   847			chip->chip_type = id->driver_data;
   848		}
   849	
   850		i2c_set_clientdata(i2c, chip);
   851		chip->dev = &i2c->dev;
   852	
   853		if (!i2c->irq) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28916 bytes --]

^ permalink raw reply

* Re: [PATCHv2] phy: cpcap-usb: Add CPCAP PMIC USB support
From: Tony Lindgren @ 2017-03-27 15:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Marcel Partap, Michael Scott
In-Reply-To: <d47b0f99-1ef8-3bff-0fbe-edb95ed41e23-l0cyMroinI0@public.gmane.org>

* Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> [170326 23:27]:
> On Thursday 23 March 2017 05:16 AM, Tony Lindgren wrote:
> > +static const struct phy_ops ops = {
> > +	.owner		= THIS_MODULE,
> > +};
> 
> Given that this phy doesn't have any phy_ops, Is there a reason for registering
> this phy with the phy framework? Is it because this driver uses the phy_core's
> pm_runtime feature?

Well there's also an ULPI PHY on CPCAP, but it's not used in the
motorola-mapphone configuration. I think some other configurations
use it though, so probably best to keep it.

And yeah, for PM runtime, I was thinking that we can have the
UART mode be the idle state.

> > +static void cpcap_usb_detect(struct work_struct *work)
> > +{
> > +	struct cpcap_phy_ddata *ddata;
> > +	struct cpcap_usb_ints_state s;
> > +	bool vbus = false;
> > +	int error;
> > +
> > +	ddata = container_of(work, struct cpcap_phy_ddata, detect_work.work);
> > +
> > +	error = cpcap_phy_get_ints_state(ddata, &s);
> > +	if (error)
> > +		return;
> > +
> > +	/* See also cpcap-charger.c phy_companion for VBUS handling */
> 
> I think this companion should have ideally used extcon framework. Then we could
> have used extcon_get_state() here.

OK I'll take a look at that. I'd like to do that as a follow-up
patches if that's OK with you.

FYI in case you're wondering, there are ADCs on the PMIC available
via IIO, and the charger is also using IIO framework for VBUS and
battery voltage. And the VBUS is set up as reverse mode in the
charger registers and the charging needs to be disabled for VBUS.

I guess Ideally the VBUS would be just a regulator from Linux point
of view that also allows negative current :)

> > +	ddata->vusb = devm_regulator_get(&pdev->dev, "vusb");
> > +	if (IS_ERR(ddata->vusb))
> > +		return PTR_ERR(ddata->vusb);
> > +
> > +	error = regulator_enable(ddata->vusb);
> > +	if (error)
> > +		return error;
> 
> Maybe we should create power_on ops and do regulator enable there?

This regulator is also needed for the UART mode, so it would have
to be disabled conditionally based on maybe a module parameter.
But yeah thanks for the pointer I did not even realize you have
power_on ops :)

Seems this can be also done when implementing PM runtime handling.
If you want some of these changes done for the initial patch,
please let me know.

Regards,

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

^ permalink raw reply

* [PATCH v4 0/4] iio: adc: Maxim max9611 driver
From: Jacopo Mondi @ 2017-03-27 15:12 UTC (permalink / raw)
  To: geert-Td1EMuHUCqxL1ZNQvxDV9g,
	wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
	magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hello,
   fourth round for max9611 driver.

Biggest shift from v3 is change in locking, now performed only at
read_single() level.
No locking in read_raw, nor in read_csa_voltage(), as the only thing to
protect is the on-chip mux configuration between and I2c write and a read.
All other functions end up in calling this read_single() one, and they do not
share variables, so concurrent accesses to sysfs attributes should be safe.

Changed scale of temperature channel, and fixed shunt resistor names, that now
appears as properties of their respective power and current channels.

Ack on dt-bindings for "shunt-resistor-ohm" property pending from device
tree guardians.

Output of iio_info on Salvator-X board for max9611 chip installed on
VDD_0.8 lines.
The VDD_0.8 line powers the CPU cluster and on-board RAM.

        iio:device0: max9611
                5 channels found:
                        voltage0:  (input)
                        1 channel-specific attributes found:
                                attr 0: input value: 4.085000000
                        voltage1:  (input)
                        3 channel-specific attributes found:
                                attr 0: scale value: 14
                                attr 1: offset value: 1
                                attr 2: raw value: 59
                        power:  (input)
                        2 channel-specific attributes found:
                                attr 0: shunt_resistor value: 5000
                                attr 1: input value: 663.404000000
                        current:  (input)
                        2 channel-specific attributes found:
                                attr 0: shunt_resistor value: 5000
                                attr 1: input value: 817.000000000
                        temp:  (input)
                        2 channel-specific attributes found:
                                attr 0: scale value: 480.076812289
                                attr 1: raw value: 57

The collected information represent:

* voltage0 (current sense voltage) Vcsa
  voltage drop between RS+ and RS- input = 4,085 mV
* voltage1: (common input voltage) Vcim
  voltage at RS+ input = (59 - 1) * 14 = 812 mV
* current flowing on shunt resistor (Icsa)
  = Vcsa / Rshunt = 817 mA
* power load on the sensed line (Pload)
  = Vcim * Icsa = 663 m
* die temperature = (57 * 480.07) = 27360 milli Celsius

Thanks
   j

v1 -> v2:
- Drop wildcard (max961x) in driver, documentation and dt-bindings. Use
  max9611 instead.
- Make 3 processed channels for csa voltage, csa current and power load
- Remove wrapper functions around i2c buffer access
- Add locking in read_raw()
- Make 2 separate attributes for shunt resistor: current and power
- Renamed shunt resistor attribute
- Fixed several review comments

v2 -> v3:
- Incorporated Geert's suggestions on using a more generic name for ADC nodes
  in device tree
- Changed "maxim,shunt-resistor-uohm" to "shunt-resistor-uohm"
- Update documentation accordingly to the two previous changes

v3 -> v4:
- Move all locking to read_single() function
- Re-structure read_raw() with Jonathan's and Peter's comments
- Rename some defines pre-pending MAX9611 prefix
- Use values from .comaptible entries for device name
- Change temperature scale to have a value in milli Celsius
- Simplify init routine using read_single

Jacopo Mondi (4):
  Documentation: dt-bindings: iio: Add max9611 ADC
  iio: Documentation: Add max9611 sysfs documentation
  iio: adc: Add Maxim max9611 ADC driver
  arm64: dts: salvator-x: Add current sense amplifiers

 .../ABI/testing/sysfs-bus-iio-adc-max9611          |  17 +
 .../devicetree/bindings/iio/adc/max9611.txt        |  26 +
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts |  18 +
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/max9611.c                          | 583 +++++++++++++++++++++
 6 files changed, 655 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
 create mode 100644 Documentation/devicetree/bindings/iio/adc/max9611.txt
 create mode 100644 drivers/iio/adc/max9611.c

-- 
2.7.4

^ permalink raw reply

* [PATCH v4 1/4] Documentation: dt-bindings: iio: Add max9611 ADC
From: Jacopo Mondi @ 2017-03-27 15:12 UTC (permalink / raw)
  To: geert-Td1EMuHUCqxL1ZNQvxDV9g,
	wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
	magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490627578-15938-1-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

Add device tree bindings documentation for Maxim max9611/max9612 current
sense amplifier.

Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/iio/adc/max9611.txt        | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/max9611.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/max9611.txt b/Documentation/devicetree/bindings/iio/adc/max9611.txt
new file mode 100644
index 0000000..8430d3b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/max9611.txt
@@ -0,0 +1,26 @@
+* Maxim max9611/max9612 current sense amplifier with 12-bits ADC interface
+
+Maxim max9611/max9612 is an high-side current sense amplifier with integrated
+12-bits ADC communicating over I2c bus.
+The device node for this driver shall be a child of a I2c controller.
+
+Required properties
+  - compatible: Should be "maxim,max9611" or "maxim,max9612"
+  - reg: The 7-bits long I2c address of the device
+  - shunt-resistor-uohm: Value, in uOhm, of the current sense shunt resistor
+
+Example:
+
+&i2c4 {
+	csa: adc@7c {
+		compatible = "maxim,max9611";
+		reg = <0x7c>;
+
+		shunt-resistor-uohm = <5000>;
+	};
+};
+
+This device node describes a current sense amplifier sitting on I2c4 bus
+with address 0x7c (read address is 0xf9, write address is 0xf8).
+A sense resistor of 0,005 Ohm is installed between RS+ and RS- current-sensing
+inputs.
-- 
2.7.4

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

^ permalink raw reply related

* [PATCH v4 2/4] iio: Documentation: Add max9611 sysfs documentation
From: Jacopo Mondi @ 2017-03-27 15:12 UTC (permalink / raw)
  To: geert-Td1EMuHUCqxL1ZNQvxDV9g,
	wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
	magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490627578-15938-1-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

Add documentation for max9611 driver.
Document attributes describing value of shunt resistor installed between
RS+ and RS- voltage sense inputs.

Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
---
 Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-max9611

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611 b/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
new file mode 100644
index 0000000..5020d00
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-max9611
@@ -0,0 +1,17 @@
+What:		/sys/bus/iio/devices/iio:deviceX/in_power_shunt_resistor
+Date:		March 2017
+KernelVersion:	4.12
+Contact:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description: 	The value of the shunt resistor used to compute power drain on
+                common input voltage pin (RS+). In micro Ohms.
+
+What:		/sys/bus/iio/devices/iio:deviceX/in_current_shunt_resistor
+Date:		March 2017
+KernelVersion:	4.12
+Contact:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description: 	The value of the shunt resistor used to compute current flowing
+                between RS+ and RS- voltage sense inputs. In micro Ohms.
+
+These attributes describe a single physical component, exposed as two distinct
+attributes as it is used to calculate two different values: power load and
+current flowing between RS+ and RS- inputs.
-- 
2.7.4

^ permalink raw reply related

* [PATCH v4 3/4] iio: adc: Add Maxim max9611 ADC driver
From: Jacopo Mondi @ 2017-03-27 15:12 UTC (permalink / raw)
  To: geert-Td1EMuHUCqxL1ZNQvxDV9g,
	wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
	magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490627578-15938-1-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

Add iio driver for Maxim max9611 and max9612 current-sense amplifiers
with 12-bits ADC interface.

Datasheet publicly available at:
https://datasheets.maximintegrated.com/en/ds/MAX9611-MAX9612.pdf

Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

---

Output of iio_info on Salvator-X board for max9611 chip installed on
VDD_0.8 lines.
The VDD_0.8 line powers the CPU cluster and on-board RAM.

        iio:device0: max9611
                5 channels found:
                        voltage0:  (input)
                        1 channel-specific attributes found:
                                attr 0: input value: 4.085000000
                        voltage1:  (input)
                        3 channel-specific attributes found:
                                attr 0: scale value: 14
                                attr 1: offset value: 1
                                attr 2: raw value: 59
                        power:  (input)
                        2 channel-specific attributes found:
                                attr 0: shunt_resistor value: 5000
                                attr 1: input value: 663.404000000
                        current:  (input)
                        2 channel-specific attributes found:
                                attr 0: shunt_resistor value: 5000
                                attr 1: input value: 817.000000000
                        temp:  (input)
                        2 channel-specific attributes found:
                                attr 0: scale value: 480.076812289
                                attr 1: raw value: 57

The collected information represent:

* voltage0 (current sense voltage) Vcsa
  voltage drop between RS+ and RS- input = 4,085 mV
* voltage1: (common input voltage) Vcim
  voltage at RS+ input = (59 - 1) * 14 = 812 mV
* current flowing on shunt resistor (Icsa)
  = Vcsa / Rshunt = 817 mA
* power load on the sensed line (Pload)
  = Vcim * Icsa = 663 m
* die temperature = (57 * 480.07) = 27360 milli Celsius
---
 drivers/iio/adc/Kconfig   |  10 +
 drivers/iio/adc/Makefile  |   1 +
 drivers/iio/adc/max9611.c | 583 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 594 insertions(+)
 create mode 100644 drivers/iio/adc/max9611.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index dedae7a..82f2e7b8 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -354,6 +354,16 @@ config MAX1363
 	  To compile this driver as a module, choose M here: the module will be
 	  called max1363.
 
+config	MAX9611
+	tristate "Maxim max9611/max9612 ADC driver"
+	depends on I2C
+	help
+	  Say yes here to build support for Maxim max9611/max9612 current sense
+	  amplifier with 12-bits ADC interface.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called max9611.
+
 config MCP320X
 	tristate "Microchip Technology MCP3x01/02/04/08"
 	depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d001262..149f979 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_LTC2485) += ltc2485.o
 obj-$(CONFIG_MAX1027) += max1027.o
 obj-$(CONFIG_MAX11100) += max11100.o
 obj-$(CONFIG_MAX1363) += max1363.o
+obj-$(CONFIG_MAX9611) += max9611.o
 obj-$(CONFIG_MCP320X) += mcp320x.o
 obj-$(CONFIG_MCP3422) += mcp3422.o
 obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
diff --git a/drivers/iio/adc/max9611.c b/drivers/iio/adc/max9611.c
new file mode 100644
index 0000000..e5aaf73
--- /dev/null
+++ b/drivers/iio/adc/max9611.c
@@ -0,0 +1,583 @@
+/*
+ * iio/adc/max9611.c
+ *
+ * Maxim max9611/max9612 high side current sense amplifier with
+ * 12-bit ADC interface.
+ *
+ * Copyright (C) 2017 Jacopo Mondi
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * This driver supports input common-mode voltage, current-sense
+ * amplifier with programmable gains and die temperature reading from
+ * Maxim max9611/max9612.
+ *
+ * Op-amp, analog comparator, and watchdog functionalities are not
+ * supported by this driver.
+ */
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+
+#define DRIVER_NAME 			"max9611"
+#define MAX9611_DEV_NAME		"max9611"
+#define MAX9612_DEV_NAME		"max9612"
+
+/* max9611 register addresses */
+#define MAX9611_REG_CSA_DATA		0x00
+#define MAX9611_REG_RS_DATA		0x02
+#define MAX9611_REG_TEMP_DATA		0x08
+#define MAX9611_REG_CTRL1		0x0a
+#define MAX9611_REG_CTRL2		0x0b
+
+/* max9611 REG1 mux configuration options */
+#define MAX9611_MUX_MASK		GENMASK(3,0)
+#define MAX9611_MUX_SENSE_1x		0x00
+#define MAX9611_MUX_SENSE_4x		0x01
+#define MAX9611_MUX_SENSE_8x		0x02
+#define MAX9611_INPUT_VOLT		0x03
+#define MAX9611_MUX_TEMP		0x06
+
+/* max9611 voltage (both csa and input) helper macros */
+#define MAX9611_VOLTAGE_SHIFT		0x04
+#define MAX9611_VOLTAGE_RAW(_r)		((_r) >> MAX9611_VOLTAGE_SHIFT)
+
+/*
+ * max9611 current sense amplifier voltage output:
+ * LSB and offset values depends on selected gain (1x, 4x, 8x)
+ *
+ * GAIN		LSB (nV)	OFFSET (LSB steps)
+ * 1x		107500		1
+ * 4x		26880		1
+ * 8x		13440		3
+ *
+ * The complete formula to calculate current sense voltage is:
+ *     (((adc_read >> 4) - offset) / ((1 / LSB) * 10^-3)
+ */
+#define MAX9611_CSA_1X_LSB_nV		107500
+#define MAX9611_CSA_4X_LSB_nV		26880
+#define MAX9611_CSA_8X_LSB_nV		13440
+
+#define MAX9611_CSA_1X_OFFS_RAW		1
+#define MAX9611_CSA_4X_OFFS_RAW		1
+#define MAX9611_CSA_8X_OFFS_RAW		3
+
+/*
+ * max9611 common input mode (CIM): LSB is 14mV, with 14mV offset at 25 C
+ *
+ * The complete formula to calculate input common voltage is:
+ *     (((adc_read >> 4) * 1000) - offset) / (1 / 14 * 1000)
+ */
+#define MAX9611_CIM_LSB_mV		14
+#define MAX9611_CIM_OFFSET_RAW		1
+
+/*
+ * max9611 temperature reading: LSB is 480 milli degrees Celsius
+ *
+ * The complete formula to calculate temperature is:
+ *     ((adc_read >> 7) * 1000) / (1 / 480 * 1000)
+ */
+#define MAX9611_TEMP_MAX_POS		0x7f80
+#define MAX9611_TEMP_MAX_NEG		0xff80
+#define MAX9611_TEMP_MIN_NEG		0xd980
+#define MAX9611_TEMP_MASK		GENMASK(7, 15)
+#define MAX9611_TEMP_SHIFT		0x07
+#define MAX9611_TEMP_RAW(_r)		((_r) >> MAX9611_TEMP_SHIFT)
+#define MAX9611_TEMP_SCALE_NUM		1000000
+#define MAX9611_TEMP_SCALE_DIV		2083
+
+struct max9611_dev {
+	struct device *dev;
+	struct i2c_client *i2c_client;
+	struct mutex lock;
+	unsigned int shunt_resistor_uohm;
+};
+
+enum max9611_conf_ids {
+	CONF_SENSE_1x,
+	CONF_SENSE_4x,
+	CONF_SENSE_8x,
+	CONF_IN_VOLT,
+	CONF_TEMP,
+};
+
+/**
+ * max9611_mux_conf - associate ADC mux configuration with register address
+ *		      where data shall be read from
+ */
+static const unsigned int max9611_mux_conf[][2] = {
+	/* CONF_SENSE_1x */
+	{ MAX9611_MUX_SENSE_1x, MAX9611_REG_CSA_DATA },
+	/* CONF_SENSE_4x */
+	{ MAX9611_MUX_SENSE_4x, MAX9611_REG_CSA_DATA },
+	/* CONF_SENSE_8x */
+	{ MAX9611_MUX_SENSE_8x, MAX9611_REG_CSA_DATA },
+	/* CONF_IN_VOLT */
+	{ MAX9611_INPUT_VOLT, MAX9611_REG_RS_DATA },
+	/* CONF_TEMP */
+	{ MAX9611_MUX_TEMP, MAX9611_REG_TEMP_DATA },
+};
+
+enum max9611_csa_gain {
+	CSA_GAIN_1x,
+	CSA_GAIN_4x,
+	CSA_GAIN_8x,
+};
+
+enum max9611_csa_gain_params {
+	CSA_GAIN_LSB_nV,
+	CSA_GAIN_OFFS_RAW,
+};
+
+/**
+ * max9611_csa_gain_conf - associate gain multiplier with LSB and
+ *			   offset values.
+ *
+ * Group together parameters associated with configurable gain
+ * on current sense amplifier path to ADC interface.
+ * Current sense read routine adjusts gain until it gets a meaningful
+ * value; use this structure to retrieve the correct LSB and offset values.
+ */
+static const unsigned int max9611_gain_conf[][2] = {
+	{ /* [0] CSA_GAIN_1x */
+		MAX9611_CSA_1X_LSB_nV,
+		MAX9611_CSA_1X_OFFS_RAW,
+	},
+	{ /* [1] CSA_GAIN_4x */
+		MAX9611_CSA_4X_LSB_nV,
+		MAX9611_CSA_4X_OFFS_RAW,
+	},
+	{ /* [2] CSA_GAIN_8x */
+		MAX9611_CSA_8X_LSB_nV,
+		MAX9611_CSA_8X_OFFS_RAW,
+	},
+};
+
+enum max9611_chan_addrs {
+	MAX9611_CHAN_VOLTAGE_INPUT,
+	MAX9611_CHAN_VOLTAGE_SENSE,
+	MAX9611_CHAN_TEMPERATURE,
+	MAX9611_CHAN_CURRENT_LOAD,
+	MAX9611_CHAN_POWER_LOAD,
+};
+
+static const struct iio_chan_spec max9611_channels[] = {
+	{
+	  .type			= IIO_TEMP,
+	  .info_mask_separate	= BIT(IIO_CHAN_INFO_RAW) |
+				  BIT(IIO_CHAN_INFO_SCALE),
+	  .address		= MAX9611_CHAN_TEMPERATURE,
+	},
+	{
+	  .type			= IIO_VOLTAGE,
+	  .info_mask_separate	= BIT(IIO_CHAN_INFO_PROCESSED),
+	  .address		= MAX9611_CHAN_VOLTAGE_SENSE,
+	  .indexed		= 1,
+	  .channel		= 0,
+	},
+	{
+	  .type			= IIO_VOLTAGE,
+	  .info_mask_separate	= BIT(IIO_CHAN_INFO_RAW)   |
+				  BIT(IIO_CHAN_INFO_SCALE) |
+				  BIT(IIO_CHAN_INFO_OFFSET),
+	  .address		= MAX9611_CHAN_VOLTAGE_INPUT,
+	  .indexed		= 1,
+	  .channel		= 1,
+	},
+	{
+	  .type			= IIO_CURRENT,
+	  .info_mask_separate	= BIT(IIO_CHAN_INFO_PROCESSED),
+	  .address		= MAX9611_CHAN_CURRENT_LOAD,
+	},
+	{
+	  .type			= IIO_POWER,
+	  .info_mask_separate	= BIT(IIO_CHAN_INFO_PROCESSED),
+	  .address		= MAX9611_CHAN_POWER_LOAD
+	},
+};
+
+/**
+ * max9611_read_single() - read a single value from ADC interface
+ *
+ * Data registers are 16 bit long, spread between two 8 bit registers
+ * with consecutive addresses.
+ * Configure ADC mux first, then read register at address "reg_addr".
+ * The smbus_read_word routine asks for 16 bits and the ADC is kind enough
+ * to return values from "reg_addr" and "reg_addr + 1" consecutively.
+ * Data are transmitted with big-endian ordering: MSB arrives first.
+ *
+ * @max9611: max9611 device
+ * @selector: index for mux and register configuration
+ * @raw_val: the value returned from ADC
+ */
+static int max9611_read_single(struct max9611_dev *max9611,
+			       enum max9611_conf_ids selector,
+			       u16 *raw_val)
+{
+	int ret;
+
+	u8 mux_conf = max9611_mux_conf[selector][0] & MAX9611_MUX_MASK;
+	u8 reg_addr = max9611_mux_conf[selector][1];
+
+	/*
+	 * Keep mutex lock held during read-write to avoid mux register
+	 * (CTRL1) re-configuration.
+	 */
+	mutex_lock(&max9611->lock);
+	ret = i2c_smbus_write_byte_data(max9611->i2c_client,
+					MAX9611_REG_CTRL1, mux_conf);
+	if (ret) {
+		dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
+			MAX9611_REG_CTRL1, mux_conf);
+		mutex_unlock(&max9611->lock);
+		return ret;
+	}
+
+	/*
+	 * need a delay here to make register configuration
+	 * stabilize. 1 msec at least, from empirical testing.
+	 */
+	usleep_range(1000, 2000);
+
+	ret = i2c_smbus_read_word_swapped(max9611->i2c_client, reg_addr);
+	if (ret < 0) {
+		dev_err(max9611->dev, "i2c read word from 0x%2x failed\n",
+			reg_addr);
+		mutex_unlock(&max9611->lock);
+		return ret;
+	}
+
+	*raw_val = ret;
+	mutex_unlock(&max9611->lock);
+
+	return 0;
+}
+
+/**
+ * max9611_read_csa_voltage() - read current sense amplifier output voltage
+ *
+ * Current sense amplifier output voltage is read through a configurable
+ * 1x, 4x or 8x gain.
+ * Start with plain 1x gain, and adjust gain control properly until a
+ * meaningful value is read from ADC output.
+ *
+ * @max9611: max9611 device
+ * @adc_raw: raw value read from ADC output
+ * @csa_gain: gain configuration option selector
+ */
+static int max9611_read_csa_voltage(struct max9611_dev *max9611,
+				    u16 *adc_raw,
+				    enum max9611_csa_gain *csa_gain)
+{
+	enum max9611_conf_ids gain_selectors[] = {
+		CONF_SENSE_1x,
+		CONF_SENSE_4x,
+		CONF_SENSE_8x
+	};
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < ARRAY_SIZE(gain_selectors); ++i) {
+		ret = max9611_read_single(max9611, gain_selectors[i], adc_raw);
+		if (ret)
+			return ret;
+
+		if (*adc_raw > 0) {
+			*csa_gain = gain_selectors[i];
+			return 0;
+		}
+	}
+
+	return -EIO;
+}
+
+static int max9611_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct max9611_dev *dev = iio_priv(indio_dev);
+	enum max9611_csa_gain gain_selector;
+	const unsigned int *csa_gain;
+	u16 adc_data;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+
+		switch (chan->address) {
+		case MAX9611_CHAN_TEMPERATURE:
+			ret = max9611_read_single(dev, CONF_TEMP,
+						  &adc_data);
+			if (ret)
+				return -EINVAL;
+
+			*val = MAX9611_TEMP_RAW(adc_data);
+			return IIO_VAL_INT;
+
+		case MAX9611_CHAN_VOLTAGE_INPUT:
+			ret = max9611_read_single(dev, CONF_IN_VOLT,
+						  &adc_data);
+			if (ret)
+				return -EINVAL;
+
+			*val = MAX9611_VOLTAGE_RAW(adc_data);
+			return IIO_VAL_INT;
+		}
+
+		break;
+
+	case IIO_CHAN_INFO_OFFSET:
+		/* MAX9611_CHAN_VOLTAGE_INPUT */
+		*val = MAX9611_CIM_OFFSET_RAW;
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+
+		switch (chan->address) {
+		case MAX9611_CHAN_TEMPERATURE:
+			*val = MAX9611_TEMP_SCALE_NUM;
+			*val2 = MAX9611_TEMP_SCALE_DIV;
+
+			return IIO_VAL_FRACTIONAL;
+
+		case MAX9611_CHAN_VOLTAGE_INPUT:
+			*val = MAX9611_CIM_LSB_mV;
+
+			return IIO_VAL_INT;
+		}
+
+		break;
+
+	case IIO_CHAN_INFO_PROCESSED:
+
+		switch (chan->address) {
+		case MAX9611_CHAN_VOLTAGE_SENSE:
+			/*
+			 * processed (mV): (raw - offset) * LSB (nV) / 10^6
+			 *
+			 * Even if max9611 can output raw csa voltage readings,
+			 * use a produced value as scale depends on gain.
+			 */
+			ret = max9611_read_csa_voltage(dev, &adc_data,
+						       &gain_selector);
+			if (ret)
+				return -EINVAL;
+
+			csa_gain = max9611_gain_conf[gain_selector];
+
+			adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
+			*val = MAX9611_VOLTAGE_RAW(adc_data) *
+			       csa_gain[CSA_GAIN_LSB_nV];
+			*val2 = 1000000;
+
+			return IIO_VAL_FRACTIONAL;
+
+		case MAX9611_CHAN_CURRENT_LOAD:
+			/* processed (mA): Vcsa (nV) / Rshunt (uOhm)  */
+			ret = max9611_read_csa_voltage(dev, &adc_data,
+						       &gain_selector);
+			if (ret)
+				return -EINVAL;
+
+			csa_gain = max9611_gain_conf[gain_selector];
+
+			adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
+			*val = MAX9611_VOLTAGE_RAW(adc_data) *
+			       csa_gain[CSA_GAIN_LSB_nV];
+			*val2 = dev->shunt_resistor_uohm;
+
+			return IIO_VAL_FRACTIONAL;
+
+		case MAX9611_CHAN_POWER_LOAD:
+			/*
+			 * processed (mW): Vin (mV) * Vcsa (uV) /
+			 *		   Rshunt (uOhm)
+			 */
+			ret = max9611_read_single(dev, CONF_IN_VOLT,
+						  &adc_data);
+			if (ret)
+				return -EINVAL;
+
+			adc_data -= MAX9611_CIM_OFFSET_RAW;
+			*val = MAX9611_VOLTAGE_RAW(adc_data) *
+			       MAX9611_CIM_LSB_mV;
+
+			ret = max9611_read_csa_voltage(dev, &adc_data,
+						       &gain_selector);
+			if (ret)
+				return -EINVAL;
+
+			csa_gain = max9611_gain_conf[gain_selector];
+
+			/* divide by 10^3 here to avoid 32bit overflow */
+			adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
+			*val *= MAX9611_VOLTAGE_RAW(adc_data) *
+				csa_gain[CSA_GAIN_LSB_nV] / 1000;
+			*val2 = dev->shunt_resistor_uohm;
+
+			return IIO_VAL_FRACTIONAL;
+		}
+
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t max9611_shunt_resistor_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct max9611_dev *max9611 = iio_priv(dev_to_iio_dev(dev));
+
+	return sprintf(buf, "%u\n", max9611->shunt_resistor_uohm);
+}
+
+static IIO_DEVICE_ATTR(in_power_shunt_resistor, 0444,
+		       max9611_shunt_resistor_show, NULL, 0);
+static IIO_DEVICE_ATTR(in_current_shunt_resistor, 0444,
+		       max9611_shunt_resistor_show, NULL, 0);
+
+static struct attribute *max9611_attributes[] = {
+	&iio_dev_attr_in_power_shunt_resistor.dev_attr.attr,
+	&iio_dev_attr_in_current_shunt_resistor.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group max9611_attribute_group = {
+	.attrs = max9611_attributes,
+};
+
+static const struct iio_info indio_info = {
+	.driver_module	= THIS_MODULE,
+	.read_raw	= max9611_read_raw,
+	.attrs		= &max9611_attribute_group,
+};
+
+static int max9611_init(struct max9611_dev *max9611)
+{
+	struct i2c_client *client = max9611->i2c_client;
+	u16 regval;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_WRITE_BYTE	|
+				     I2C_FUNC_SMBUS_READ_WORD_DATA)) {
+		dev_err(max9611->dev,
+			"I2c adapter does not support smbus write_byte or read_word functionalities: aborting probe.\n");
+		return -EINVAL;
+	}
+
+	/* Make sure die temperature is in range to test communications. */
+	ret = max9611_read_single(max9611, CONF_TEMP, &regval);
+	if (ret)
+		return ret;
+
+	regval = ret & MAX9611_TEMP_MASK;
+
+	if ((regval > MAX9611_TEMP_MAX_POS &&
+	     regval < MAX9611_TEMP_MIN_NEG) ||
+	     regval > MAX9611_TEMP_MAX_NEG) {
+		dev_err(max9611->dev,
+			"Invalid value received from ADC 0x%4x: aborting\n",
+			regval);
+		return -EIO;
+	}
+
+	/* Mux shall be zeroed back before applying other configurations */
+	ret = i2c_smbus_write_byte_data(max9611->i2c_client,
+					MAX9611_REG_CTRL1, 0);
+	if (ret) {
+		dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
+			MAX9611_REG_CTRL1, 0);
+		return ret;
+	}
+
+	ret = i2c_smbus_write_byte_data(max9611->i2c_client,
+					MAX9611_REG_CTRL2, 0);
+	if (ret) {
+		dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
+			MAX9611_REG_CTRL2, 0);
+		return ret;
+	}
+	usleep_range(1000, 2000);
+
+	return 0;
+}
+
+static const struct of_device_id max9611_of_table[] = {
+	{.compatible = "maxim,max9611", .data = MAX9611_DEV_NAME},
+	{.compatible = "maxim,max9612", .data = MAX9612_DEV_NAME},
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, max9611_of_table);
+static int max9611_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	const char * const shunt_res_prop = "shunt-resistor-uohm";
+	const struct device_node *of_node = client->dev.of_node;
+	const struct of_device_id *of_id =
+		of_match_device(max9611_of_table, &client->dev);
+	struct max9611_dev *max9611;
+	struct iio_dev *indio_dev;
+	unsigned int of_shunt;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*max9611));
+	if (IS_ERR(indio_dev))
+		return PTR_ERR(indio_dev);
+
+	i2c_set_clientdata(client, indio_dev);
+
+	max9611			= iio_priv(indio_dev);
+	max9611->dev		= &client->dev;
+	max9611->i2c_client	= client;
+	mutex_init(&max9611->lock);
+
+	ret = of_property_read_u32(of_node, shunt_res_prop, &of_shunt);
+	if (ret) {
+		dev_err(&client->dev,
+			"Missing %s property for %s node\n",
+			shunt_res_prop, of_node->full_name);
+		return ret;
+	}
+	max9611->shunt_resistor_uohm = of_shunt;
+
+	ret = max9611_init(max9611);
+	if (ret)
+		return ret;
+
+	indio_dev->dev.parent	= &client->dev;
+	indio_dev->dev.of_node	= client->dev.of_node;
+	indio_dev->name		= of_id->data;
+	indio_dev->modes	= INDIO_DIRECT_MODE;
+	indio_dev->info		= &indio_info;
+	indio_dev->channels	= max9611_channels;
+	indio_dev->num_channels	= ARRAY_SIZE(max9611_channels);
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static struct i2c_driver max9611_driver = {
+	.driver = {
+		   .name = DRIVER_NAME,
+		   .owner = THIS_MODULE,
+		   .of_match_table = max9611_of_table,
+	},
+	.probe = max9611_probe,
+};
+module_i2c_driver(max9611_driver);
+
+MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>");
+MODULE_DESCRIPTION("Maxim max9611/12 current sense amplifier with 12bit ADC");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

^ permalink raw reply related

* [PATCH v4 4/4] arm64: dts: salvator-x: Add current sense amplifiers
From: Jacopo Mondi @ 2017-03-27 15:12 UTC (permalink / raw)
  To: geert, wsa+renesas, magnus.damm, laurent.pinchart, jic23,
	knaack.h, lars, pmeerw, robh+dt, mark.rutland
  Cc: linux-iio, linux-renesas-soc, devicetree
In-Reply-To: <1490627578-15938-1-git-send-email-jacopo+renesas@jmondi.org>

Add device nodes for two Maxim max961x current sense amplifiers
sensing VDD_08 and DVFS_08 lines.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
index c7f40f8..389a177 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
@@ -257,6 +257,24 @@
 	status = "okay";
 };
 
+&i2c4 {
+	status = "okay";
+
+	csa_vdd: adc@7c {
+		compatible = "maxim,max9611";
+		reg = <0x7c>;
+
+		shunt-resistor-uohm = <5000>;
+	};
+
+	csa_dvfs: adc@7f {
+		compatible = "maxim,max9611";
+		reg = <0x7f>;
+
+		shunt-resistor-uohm = <5000>;
+	};
+};
+
 &wdt0 {
 	timeout-sec = <60>;
 	status = "okay";
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH RFC v4 06/10] net: qualcomm: make qca_common a separate kernel module
From: Dan Williams @ 2017-03-27 15:44 UTC (permalink / raw)
  To: Stefan Wahren, Rob Herring, Mark Rutland, David S. Miller
  Cc: Greg Kroah-Hartman, Jiri Slaby, Marcel Holtmann,
	Sebastian Reichel, netdev, devicetree, linux-serial, linux-kernel
In-Reply-To: <1490621848-24828-7-git-send-email-stefan.wahren@i2se.com>

On Mon, 2017-03-27 at 15:37 +0200, Stefan Wahren wrote:
> In order to share common functions between QCA7000 SPI and UART
> protocol
> driver the qca_common needs to be a separate kernel module.

Maybe "qca_eth_common"?  There are many things Qualcomm, slightly fewer
things Qualcomm Atheros, and "qca_common" isn't very descriptive.

Dan

> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  drivers/net/ethernet/qualcomm/Kconfig      |  8 +++++++-
>  drivers/net/ethernet/qualcomm/Makefile     |  5 +++--
>  drivers/net/ethernet/qualcomm/qca_common.c | 10 ++++++++++
>  3 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qualcomm/Kconfig
> b/drivers/net/ethernet/qualcomm/Kconfig
> index d7720bf..b4c369d 100644
> --- a/drivers/net/ethernet/qualcomm/Kconfig
> +++ b/drivers/net/ethernet/qualcomm/Kconfig
> @@ -16,7 +16,13 @@ config NET_VENDOR_QUALCOMM
>  if NET_VENDOR_QUALCOMM
>  
>  config QCA7000
> -	tristate "Qualcomm Atheros QCA7000 support"
> +	tristate
> +	help
> +	  This enables support for the Qualcomm Atheros QCA7000.
> +
> +config QCA7000_SPI
> +	tristate "Qualcomm Atheros QCA7000 SPI support"
> +	select QCA7000
>  	depends on SPI_MASTER && OF
>  	---help---
>  	  This SPI protocol driver supports the Qualcomm Atheros
> QCA7000.
> diff --git a/drivers/net/ethernet/qualcomm/Makefile
> b/drivers/net/ethernet/qualcomm/Makefile
> index 8080570..00d8729 100644
> --- a/drivers/net/ethernet/qualcomm/Makefile
> +++ b/drivers/net/ethernet/qualcomm/Makefile
> @@ -2,7 +2,8 @@
>  # Makefile for the Qualcomm network device drivers.
>  #
>  
> -obj-$(CONFIG_QCA7000) += qcaspi.o
> -qcaspi-objs := qca_spi.o qca_common.o qca_7k.o qca_debug.o
> +obj-$(CONFIG_QCA7000) += qca_common.o
> +obj-$(CONFIG_QCA7000_SPI) += qcaspi.o
> +qcaspi-objs := qca_7k.o qca_debug.o qca_spi.o
>  
>  obj-y += emac/
> diff --git a/drivers/net/ethernet/qualcomm/qca_common.c
> b/drivers/net/ethernet/qualcomm/qca_common.c
> index d930524..f2c9e76 100644
> --- a/drivers/net/ethernet/qualcomm/qca_common.c
> +++ b/drivers/net/ethernet/qualcomm/qca_common.c
> @@ -21,7 +21,9 @@
>   *   by an atheros frame while transmitted over a serial channel;
>   */
>  
> +#include <linux/init.h>
>  #include <linux/kernel.h>
> +#include <linux/module.h>
>  
>  #include "qca_common.h"
>  
> @@ -46,6 +48,7 @@ qcafrm_create_header(u8 *buf, u16 length)
>  
>  	return QCAFRM_HEADER_LEN;
>  }
> +EXPORT_SYMBOL_GPL(qcafrm_create_header);
>  
>  u16
>  qcafrm_create_footer(u8 *buf)
> @@ -57,6 +60,7 @@ qcafrm_create_footer(u8 *buf)
>  	buf[1] = 0x55;
>  	return QCAFRM_FOOTER_LEN;
>  }
> +EXPORT_SYMBOL_GPL(qcafrm_create_footer);
>  
>  /*   Gather received bytes and try to extract a full ethernet frame
> by
>   *   following a simple state machine.
> @@ -154,3 +158,9 @@ qcafrm_fsm_decode(struct qcafrm_handle *handle,
> u8 *buf, u16 buf_len, u8 recv_by
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(qcafrm_fsm_decode);
> +
> +MODULE_DESCRIPTION("Qualcomm Atheros Common");
> +MODULE_AUTHOR("Qualcomm Atheros Communications");
> +MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
> +MODULE_LICENSE("Dual BSD/GPL");

^ permalink raw reply

* [RFC PATCH 1/2] dt-bindings: mtd: Add Cavium SOCs NAND bindings
From: Jan Glauber @ 2017-03-27 16:05 UTC (permalink / raw)
  To: Boris Brezillon, Richard Weinberger, David Woodhouse,
	Brian Norris, Marek Vasut, Cyrille Pitchen
  Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jan Glauber,
	Rob Herring, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170327160524.29019-1-jglauber-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

Add device tree binding description for Cavium SOC nand flash controller.

CC: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
CC: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
CC: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Signed-off-by: Jan Glauber <jglauber-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/mtd/cavium_nand.txt        | 32 ++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/cavium_nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/cavium_nand.txt b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
new file mode 100644
index 0000000..4698d1f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/cavium_nand.txt
@@ -0,0 +1,32 @@
+* Cavium NAND controller
+
+Required properties:
+
+- compatible:		should be "cavium,cn8xxx-nand"
+- reg:			PCI devfn
+- clocks:		must contain system clock
+- #address-cells:	<1>
+- #size-cells:		<0>
+
+The nand flash controller may contain up to 8 subnodes representing
+NAND flash chips. Their properties are as follows.
+
+Required properties:
+- compatible:		should be "cavium,nandcs"
+- reg:			a single integer representing the chip-select number
+- nand-ecc-mode:	see nand.txt
+
+Example:
+
+nfc: nand@b,0 {
+	compatible = "cavium,cn8xxx-nand";
+	reg = <0x5800 0 0 0 0>;
+	clocks = <&sclk>;
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	nand@1 {
+		compatible = "cavium,nandcs";
+		reg = <1>;
+		nand-ecc-mode = "on-die";
+};
-- 
2.9.0.rc0.21.g7777322

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

^ permalink raw reply related

* [PATCH] ARM: dts: imx6qp-nitrogen6_som2: add Quad Plus variant of the SOM
From: Gary Bisson @ 2017-03-27 16:06 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
	Gary Bisson

https://boundarydevices.com/product/nit6x-som-v2/

Signed-off-by: Gary Bisson <gary.bisson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
---
 arch/arm/boot/dts/Makefile                  |  1 +
 arch/arm/boot/dts/imx6qp-nitrogen6_som2.dts | 55 +++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx6qp-nitrogen6_som2.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 011808490fed..6190afe94371 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -426,6 +426,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
 	imx6q-wandboard.dtb \
 	imx6q-wandboard-revb1.dtb \
 	imx6qp-nitrogen6_max.dtb \
+	imx6qp-nitrogen6_som2.dtb \
 	imx6qp-sabreauto.dtb \
 	imx6qp-sabresd.dtb
 dtb-$(CONFIG_SOC_IMX6SL) += \
diff --git a/arch/arm/boot/dts/imx6qp-nitrogen6_som2.dts b/arch/arm/boot/dts/imx6qp-nitrogen6_som2.dts
new file mode 100644
index 000000000000..011726c836cd
--- /dev/null
+++ b/arch/arm/boot/dts/imx6qp-nitrogen6_som2.dts
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2017 Boundary Devices, Inc.
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file 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.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+
+#include "imx6qp.dtsi"
+#include "imx6qdl-nitrogen6_som2.dtsi"
+
+/ {
+	model = "Boundary Devices i.MX6 Quad Plus Nitrogen6_SOM2 Board";
+	compatible = "boundary,imx6qp-nitrogen6_som2", "fsl,imx6qp";
+};
+
+&sata {
+	status = "okay";
+};
-- 
2.11.0

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

^ permalink raw reply related

* Re: [PATCH 1/7] Documentation: dt: net: Update the ath9k binding for SoC devices
From: Christian Lamparter @ 2017-03-27 16:11 UTC (permalink / raw)
  To: Alban
  Cc: QCA ath9k Development, John Crispin, Kalle Valo, Rob Herring,
	Mark Rutland, linux-wireless, netdev, devicetree, linux-kernel, m
In-Reply-To: <1489439116-4233-1-git-send-email-albeu@free.fr>

On Monday, March 13, 2017 10:05:09 PM CEST Alban wrote:
> The current binding only cover PCI devices so extend it for SoC devices.
> 
> Most SoC platforms use an MTD partition for the calibration data
> instead of an EEPROM. The qca,no-eeprom property was added to allow
> loading the EEPROM content using firmware loading. This new binding
> replace this hack with NVMEM cells, so we also mark the qca,no-eeprom
> property as deprecated in case anyone ever used it.

Please don't mark "qca,no-eeprom" as deprecated then.
If some devices geniously need to rely on userspace for extracting 
and processing the calibration data, it should be stay a
optional properties.

For example: A device that can't do easily without "qca,no-eeprom" is
the AVM FRITZ!WLAN Repeater 300E. For this device, the caldata 
is stored in the flash, however for whatever reason the vendor
choose to "reverse" it. (like completely back to front, not byteswapped
or something). So an extra "unreversing step" is required. So, it would
require some sort of a special nvmem-provider-processor as an 
alternative.

> Signed-off-by: Alban <albeu@free.fr>
> ---
> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> index b7396c8..61f5f6d 100644
> --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.txt
> @@ -27,16 +27,34 @@ Required properties:
>  			- 0034 for AR9462
>  			- 0036 for AR9565
>  			- 0037 for AR9485
> +		For SoC devices the compatible should be "qca,<soctype>-wmac"
> +		and one of the following fallbacks:
> +			- "qca,ar9100-wmac"
> +			- "qca,ar9330-wmac"
> +			- "qca,ar9340-wmac"
> +			- "qca,qca9550-wmac"
> +			- "qca,qca9530-wmac"
>  - reg: Address and length of the register set for the device.
>  
> +Required properties for SoC devices:
> +- interrupt-parent: phandle of the parent interrupt controller.
> +- interrupts: Interrupt specifier for the controllers interrupt.
> +
>  Optional properties:
> +- mac-address: See ethernet.txt in the parent directory
> +- local-mac-address: See ethernet.txt in the parent directory
> +- clock-names: has to be "ref"
> +- clocks: phandle of the reference clock
> +- resets: phandle of the reset line
> +- nvmem-cell-names: has to be "eeprom" and/or "address"
> +- nvmem-cells: phandle to the eeprom nvmem cell and/or to the mac address
> +		nvmem cell.
> +
> +Deprecated properties:
>  - qca,no-eeprom: Indicates that there is no physical EEPROM connected to the
>  			ath9k wireless chip (in this case the calibration /
>  			EEPROM data will be loaded from userspace using the
>  			kernel firmware loader).
> -- mac-address: See ethernet.txt in the parent directory
> -- local-mac-address: See ethernet.txt in the parent directory
> -
It sounds like you want to deprecate mac-address and local-mac-address as well.
If so you sould add this to the commit as well. From my point of view, people
mostly flat-out patched the eeprom-image if they wanted to set the mac-address.
However, this was an extra step, if nvmem does away with it, I'm completely
fine with deprecating these properties.

Thanks,
Christian

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx6qp-nitrogen6_som2: add Quad Plus variant of the SOM
From: Fabio Estevam @ 2017-03-27 16:25 UTC (permalink / raw)
  To: Gary Bisson
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Shawn Guo
In-Reply-To: <20170327160601.12871-1-gary.bisson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>

On Mon, Mar 27, 2017 at 1:06 PM, Gary Bisson
<gary.bisson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org> wrote:
> https://boundarydevices.com/product/nit6x-som-v2/
>
> Signed-off-by: Gary Bisson <gary.bisson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>

Reviewed-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 6/9] coresight: add support for CPU debug module
From: Suzuki K Poulose @ 2017-03-27 16:34 UTC (permalink / raw)
  To: Leo Yan, Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown,
	Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
	John Stultz, linux-doc, linux-kernel, devicetree,
	linux-arm-kernel, linux-arm-msm, linux-soc, linux-clk, mike.leach,
	sudeep.ho
In-Reply-To: <1490466197-29163-7-git-send-email-leo.yan@linaro.org>

On 25/03/17 18:23, Leo Yan wrote:
> Coresight includes debug module and usually the module connects with CPU
> debug logic. ARMv8 architecture reference manual (ARM DDI 0487A.k) has
> description for related info in "Part H: External Debug".
>
> Chapter H7 "The Sample-based Profiling Extension" introduces several
> sampling registers, e.g. we can check program counter value with
> combined CPU exception level, secure state, etc. So this is helpful for
> analysis CPU lockup scenarios, e.g. if one CPU has run into infinite
> loop with IRQ disabled. In this case the CPU cannot switch context and
> handle any interrupt (including IPIs), as the result it cannot handle
> SMP call for stack dump.
>
> This patch is to enable coresight debug module, so firstly this driver
> is to bind apb clock for debug module and this is to ensure the debug
> module can be accessed from program or external debugger. And the driver
> uses sample-based registers for debug purpose, e.g. when system triggers
> panic, the driver will dump program counter and combined context
> registers (EDCIDSR, EDVIDSR); by parsing context registers so can
> quickly get to know CPU secure state, exception level, etc.
>
> Some of the debug module registers are located in CPU power domain, so
> this requires the CPU power domain stays on when access related debug
> registers, but the power management for CPU power domain is quite
> dependent on SoC integration for power management. For the platforms
> which with sane power controller implementations, this driver follows
> the method to set EDPRCR to try to pull the CPU out of low power state
> and then set 'no power down request' bit so the CPU has no chance to
> lose power.
>
> If the SoC has not followed up this design well for power management
> controller, the driver introduces module parameter "idle_constraint".
> Setting this parameter for latency requirement in microseconds, finally
> we can constrain all or partial idle states to ensure the CPU power
> domain is enabled, this is a backup method to access coresight CPU
> debug component safely.

Leo,

Thanks a lot for the quick rework. I don't fully understand (yet!) why we need the
idle_constraint. I will leave it for Sudeep to comment on it, as he is the expert
in that area. Some minor comments below.

>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  drivers/hwtracing/coresight/Kconfig               |  11 +
>  drivers/hwtracing/coresight/Makefile              |   1 +
>  drivers/hwtracing/coresight/coresight-cpu-debug.c | 704 ++++++++++++++++++++++
>  3 files changed, 716 insertions(+)
>  create mode 100644 drivers/hwtracing/coresight/coresight-cpu-debug.c
>
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index 130cb21..18d7931 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -89,4 +89,15 @@ config CORESIGHT_STM
>  	  logging useful software events or data coming from various entities
>  	  in the system, possibly running different OSs
>
> +config CORESIGHT_CPU_DEBUG
> +	tristate "CoreSight CPU Debug driver"
> +	depends on ARM || ARM64
> +	depends on DEBUG_FS
> +	help
> +	  This driver provides support for coresight debugging module. This
> +	  is primarily used to dump sample-based profiling registers when
> +	  system triggers panic, the driver will parse context registers so
> +	  can quickly get to know program counter (PC), secure state,
> +	  exception level, etc.

May be we should mention/warn the user about the possible caveats of using
this feature to help him make a better decision ? And / Or we should add a documentation
for it. We have collected some real good information over the discussions and
it is a good idea to capture it somewhere.

> +
>  endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index af480d9..433d590 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
>  					coresight-etm4x-sysfs.o
>  obj-$(CONFIG_CORESIGHT_QCOM_REPLICATOR) += coresight-replicator-qcom.o
>  obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
> +obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
> diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c
> new file mode 100644
> index 0000000..fbec1d1
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c

> +#define EDPCSR				0x0A0
> +#define EDCIDSR				0x0A4
> +#define EDVIDSR				0x0A8
> +#define EDPCSR_HI			0x0AC
> +#define EDOSLAR				0x300
> +#define EDPRCR				0x310
> +#define EDPRSR				0x314
> +#define EDDEVID1			0xFC4
> +#define EDDEVID				0xFC8
> +
> +#define EDPCSR_PROHIBITED		0xFFFFFFFF
> +
> +/* bits definition for EDPCSR */
> +#ifndef CONFIG_64BIT

We don't need this to protect the defintions, see comments around adjust_pc method.

> +#define EDPCSR_THUMB			BIT(0)
> +#define EDPCSR_ARM_INST_MASK		GENMASK(31, 2)
> +#define EDPCSR_THUMB_INST_MASK		GENMASK(31, 1)
> +#endif
> +
> +/* bits definition for EDPRCR */
> +#define EDPRCR_COREPURQ			BIT(3)
> +#define EDPRCR_CORENPDRQ		BIT(0)
> +
> +/* bits definition for EDPRSR */
> +#define EDPRSR_DLK			BIT(6)
> +#define EDPRSR_PU			BIT(0)
> +
> +/* bits definition for EDVIDSR */
> +#define EDVIDSR_NS			BIT(31)
> +#define EDVIDSR_E2			BIT(30)
> +#define EDVIDSR_E3			BIT(29)
> +#define EDVIDSR_HV			BIT(28)
> +#define EDVIDSR_VMID			GENMASK(7, 0)
> +
> +/*
> + * bits definition for EDDEVID1:PSCROffset
> + *
> + * NOTE: armv8 and armv7 have different definition for the register,
> + * so consolidate the bits definition as below:
> + *
> + * 0b0000 - Sample offset applies based on the instruction state, we
> + *          rely on EDDEVID to check if EDPCSR is implemented or not
> + * 0b0001 - No offset applies.
> + * 0b0010 - No offset applies, but do not use in AArch32 mode
> + *
> + */
> +#define EDDEVID1_PCSR_OFFSET_MASK	GENMASK(3, 0)
> +#define EDDEVID1_PCSR_OFFSET_INS_SET	(0x0)
> +#define EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32	(0x2)
> +
> +/* bits definition for EDDEVID */
> +#define EDDEVID_PCSAMPLE_MODE		GENMASK(3, 0)
> +#define EDDEVID_IMPL_NONE		(0x0)
> +#define EDDEVID_IMPL_EDPCSR		(0x1)
> +#define EDDEVID_IMPL_EDPCSR_EDCIDSR	(0x2)
> +#define EDDEVID_IMPL_FULL		(0x3)
> +
> +#define DEBUG_WAIT_TIMEOUT		32
> +
> +struct debug_drvdata {
> +	void __iomem	*base;
> +	struct device	*dev;
> +	int		cpu;
> +
> +	bool		edpcsr_present;
> +	bool		edcidsr_present;
> +	bool		edvidsr_present;
> +	bool		pc_has_offset;
> +

> +	u32		eddevid;
> +	u32		eddevid1;

We don't need those two registers once we initialise the bool flags above.
So, we could as well drop them from here.

> +
> +	u32		edpcsr;
> +	u32		edpcsr_hi;

> +	u32		edprcr;

Unused member ?

> +	u32		edprsr;
> +	u32		edvidsr;
> +	u32		edcidsr;
> +};
> +
> +static DEFINE_MUTEX(debug_lock);
> +static DEFINE_PER_CPU(struct debug_drvdata *, debug_drvdata);
> +static int debug_count;
> +static struct dentry *debug_debugfs_dir;
> +
> +static struct pm_qos_request debug_qos_req;
> +static int idle_constraint = PM_QOS_DEFAULT_VALUE;
> +module_param(idle_constraint, int, 0600);
> +MODULE_PARM_DESC(idle_constraint, "Latency requirement in microseconds for CPU "
> +		 "idle states (default is -1, which means have no limiation "
> +		 "to CPU idle states; 0 means disabling all idle states; user "
> +		 "can choose other platform dependent values so can disable "
> +		 "specific idle states for the platform)");

Correct me if I am wrong,

All we want to do is disable the CPUIdle explicitly if the user knows that this
could be a problem to use CPU debug on his platform. So, in effect, we should
only be using idle_constraint = 0 or -1.

In which case, we could make it easier for the user to tell us, either

  0 - Don't do anything with CPUIdle (default)
  1 - Disable CPUIdle for me as I know the platform has issues with CPU debug and CPUidle.

than explaining the miscrosecond latency etc and make the appropriate calls underneath.
something like (not necessarily the same name) :

module_param(broken_with_cpuidle, bool, 0600);
MODULE_PARAM_DESC(broken_with_cpuidle, "Specifies whether the CPU debug has issues with CPUIdle on"
				       " the platform. Non-zero value implies CPUIdle has to be"
				       " explicitly disabled.",);

> +
> +static bool debug_enable;
> +module_param_named(enable, debug_enable, bool, 0600);
> +MODULE_PARM_DESC(enable, "Knob to enable debug functionality "
> +		 "(default is 0, which means is disabled by default)");

> +static void debug_force_cpu_powered_up(struct debug_drvdata *drvdata)
> +{
> +	int timeout = DEBUG_WAIT_TIMEOUT;
> +
> +	drvdata->edprsr = readl_relaxed(drvdata->base + EDPRSR);
> +
> +	CS_UNLOCK(drvdata->base);
> +
> +	/* Bail out if CPU is powered up yet */
> +	if (drvdata->edprsr & EDPRSR_PU)
> +		goto out_powered_up;
> +
> +	/*
> +	 * Send request to power management controller and assert
> +	 * DBGPWRUPREQ signal; if power management controller has
> +	 * sane implementation, it should enable CPU power domain
> +	 * in case CPU is in low power state.
> +	 */
> +	drvdata->edprsr = readl(drvdata->base + EDPRCR);
> +	drvdata->edprsr |= EDPRCR_COREPURQ;

You seem to be overloading the edprsr member here with EDPRCR by mistake.
Since we don't need a cached value of EDPRCR, you might as well use a local
variable here.

> +	writel(drvdata->edprsr, drvdata->base + EDPRCR);
> +
> +	/* Wait for CPU to be powered up (timeout~=32ms) */
> +	while (timeout--) {
> +		drvdata->edprsr = readl_relaxed(drvdata->base + EDPRSR);
> +		if (drvdata->edprsr & EDPRSR_PU)
> +			break;
> +
> +		usleep_range(1000, 2000);
> +	}

We have coresight_timeout() already, but not in a reusable shape (regarding
the timeout). We could possibly reuse it in the future.

> +
> +	/*
> +	 * Unfortunately the CPU cannot be powered up, so return
> +	 * back and later has no permission to access other
> +	 * registers. For this case, should set 'idle_constraint'
> +	 * to ensure CPU power domain is enabled!
> +	 */
> +	if (!(drvdata->edprsr & EDPRSR_PU)) {
> +		pr_err("%s: power up request for CPU%d failed\n",
> +			__func__, drvdata->cpu);
> +		goto out;
> +	}
> +
> +out_powered_up:
> +	debug_os_unlock(drvdata);

Question: Do we need a matching debug_os_lock() once we are done ?

> +
> +	/*
> +	 * At this point the CPU is powered up, so set the no powerdown
> +	 * request bit so we don't lose power and emulate power down.
> +	 */
> +	drvdata->edprsr = readl(drvdata->base + EDPRCR);
> +	drvdata->edprsr |= EDPRCR_COREPURQ | EDPRCR_CORENPDRQ;
> +	writel(drvdata->edprsr, drvdata->base + EDPRCR);
> +
> +out:
> +	CS_LOCK(drvdata->base);
> +}
> +
> +static void debug_read_regs(struct debug_drvdata *drvdata)
> +{
> +	/*
> +	 * Ensure CPU power domain is enabled to let registers
> +	 * are accessiable.
> +	 */
> +	debug_force_cpu_powered_up(drvdata);
> +
> +	if (!debug_access_permitted(drvdata))
> +		return;
> +
> +	CS_UNLOCK(drvdata->base);
> +
> +	debug_os_unlock(drvdata);
> +
> +	drvdata->edpcsr = readl_relaxed(drvdata->base + EDPCSR);
> +
> +	/*
> +	 * As described in ARM DDI 0487A.k, if the processing
> +	 * element (PE) is in debug state, or sample-based
> +	 * profiling is prohibited, EDPCSR reads as 0xFFFFFFFF;
> +	 * EDCIDSR, EDVIDSR and EDPCSR_HI registers also become
> +	 * UNKNOWN state. So directly bail out for this case.
> +	 */
> +	if (drvdata->edpcsr == EDPCSR_PROHIBITED)
> +		goto out;
> +
> +	/*
> +	 * A read of the EDPCSR normally has the side-effect of
> +	 * indirectly writing to EDCIDSR, EDVIDSR and EDPCSR_HI;
> +	 * at this point it's safe to read value from them.
> +	 */
> +	if (IS_ENABLED(CONFIG_64BIT))
> +		drvdata->edpcsr_hi = readl_relaxed(drvdata->base + EDPCSR_HI);
> +
> +	if (drvdata->edcidsr_present)
> +		drvdata->edcidsr = readl_relaxed(drvdata->base + EDCIDSR);
> +
> +	if (drvdata->edvidsr_present)
> +		drvdata->edvidsr = readl_relaxed(drvdata->base + EDVIDSR);
> +
> +out:
> +	CS_LOCK(drvdata->base);
> +}
> +

> +#ifndef CONFIG_64BIT

Instead of using this #ifndef/ifdef check twice (here and in the caller), we could :

> +static unsigned long debug_adjust_pc(struct debug_drvdata *drvdata,
> +				     unsigned long pc)
> +{
> +	unsigned long arm_inst_offset = 0, thumb_inst_offset = 0;
> +
	if (IS_ENABLED(CONFIG_64BIT))
		return drvdata->edpcsr_hi << 32 | drvdata->edpcsr;

> +	if (drvdata->pc_has_offset) {
> +		arm_inst_offset = 8;
> +		thumb_inst_offset = 4;
> +	}
> +
> +	/* Handle thumb instruction */
> +	if (pc & EDPCSR_THUMB) {
> +		pc = (pc & EDPCSR_THUMB_INST_MASK) - thumb_inst_offset;
> +		return pc;
> +	}
> +
> +	/*
> +	 * Handle arm instruction offset, if the arm instruction
> +	 * is not 4 byte alignment then it's possible the case
> +	 * for implementation defined; keep original value for this
> +	 * case and print info for notice.
> +	 */
> +	if (pc & BIT(1))
> +		pr_emerg("Instruction offset is implementation defined\n");
> +	else
> +		pc = (pc & EDPCSR_ARM_INST_MASK) - arm_inst_offset;
> +
> +	return pc;
> +}

> +#endif
> +
> +static void debug_dump_regs(struct debug_drvdata *drvdata)
> +{
> +	unsigned long pc;
> +
> +	pr_emerg("\tEDPRSR:  %08x (Power:%s DLK:%s)\n", drvdata->edprsr,
> +		 drvdata->edprsr & EDPRSR_PU ? "On" : "Off",
> +		 drvdata->edprsr & EDPRSR_DLK ? "Lock" : "Unlock");
> +
> +	if (!debug_access_permitted(drvdata)) {
> +		pr_emerg("No permission to access debug registers!\n");
> +		return;
> +	}
> +
> +	if (drvdata->edpcsr == EDPCSR_PROHIBITED) {
> +		pr_emerg("CPU is in Debug state or profiling is prohibited!\n");
> +		return;
> +	}
> +

> +#ifdef CONFIG_64BIT
> +	pc = (unsigned long)drvdata->edpcsr_hi << 32 |
> +	     (unsigned long)drvdata->edpcsr;
> +#else
> +	pc = debug_adjust_pc(drvdata, (unsigned long)drvdata->edpcsr);
> +#endif

nit: see above, comment for debug_adjust_pc().


> +
> +	pr_emerg("\tEDPCSR:  [<%p>] %pS\n", (void *)pc, (void *)pc);
> +
> +	if (drvdata->edcidsr_present)
> +		pr_emerg("\tEDCIDSR: %08x\n", drvdata->edcidsr);
> +
> +	if (drvdata->edvidsr_present)
> +		pr_emerg("\tEDVIDSR: %08x (State:%s Mode:%s Width:%dbits VMID:%x)\n",
> +			 drvdata->edvidsr,
> +			 drvdata->edvidsr & EDVIDSR_NS ? "Non-secure" : "Secure",
> +			 drvdata->edvidsr & EDVIDSR_E3 ? "EL3" :
> +				(drvdata->edvidsr & EDVIDSR_E2 ? "EL2" : "EL1/0"),
> +			 drvdata->edvidsr & EDVIDSR_HV ? 64 : 32,
> +			 drvdata->edvidsr & (u32)EDVIDSR_VMID);
> +}
> +
> +static void debug_init_arch_data(void *info)
> +{
> +	struct debug_drvdata *drvdata = info;
> +	u32 mode, pcsr_offset;
> +
> +	CS_UNLOCK(drvdata->base);
> +
> +	debug_os_unlock(drvdata);
> +
> +	/* Read device info */
> +	drvdata->eddevid  = readl_relaxed(drvdata->base + EDDEVID);
> +	drvdata->eddevid1 = readl_relaxed(drvdata->base + EDDEVID1);

As mentioned above, both of these registers are only need at init time to
figure out the flags we set here. So we could remove them.

> +
> +	CS_LOCK(drvdata->base);
> +
> +	/* Parse implementation feature */
> +	mode = drvdata->eddevid & EDDEVID_PCSAMPLE_MODE;
> +	pcsr_offset = drvdata->eddevid1 & EDDEVID1_PCSR_OFFSET_MASK;


> +
> +	if (mode == EDDEVID_IMPL_NONE) {
> +		drvdata->edpcsr_present  = false;
> +		drvdata->edcidsr_present = false;
> +		drvdata->edvidsr_present = false;
> +	} else if (mode == EDDEVID_IMPL_EDPCSR) {
> +		drvdata->edpcsr_present  = true;
> +		drvdata->edcidsr_present = false;
> +		drvdata->edvidsr_present = false;
> +	} else if (mode == EDDEVID_IMPL_EDPCSR_EDCIDSR) {
> +		if (!IS_ENABLED(CONFIG_64BIT) &&
> +			(pcsr_offset == EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32))
> +			drvdata->edpcsr_present = false;
> +		else
> +			drvdata->edpcsr_present = true;

Sorry, I forgot why we do this check only in this mode. Shouldn't this be
common to all modes (of course which implies PCSR is present) ?

> +
> +		drvdata->edcidsr_present = true;
> +		drvdata->edvidsr_present = false;
> +	} else if (mode == EDDEVID_IMPL_FULL) {
> +		drvdata->edpcsr_present  = true;
> +		drvdata->edcidsr_present = true;
> +		drvdata->edvidsr_present = true;
> +	}
> +
> +	if (IS_ENABLED(CONFIG_64BIT))
> +		drvdata->pc_has_offset = false;
> +	else
> +		drvdata->pc_has_offset =
> +			(pcsr_offset == EDDEVID1_PCSR_OFFSET_INS_SET);
> +

nit: This if-else chain could be replaced by :

	
	drvdata->edpcsr_present = false;
	drvdata->edcidsr_present = false;
	drvdata->edvidsr_present = false;

	switch(mode) {
	case EDDEVID_IMPL_FULL:
		drvdata->edvidsr_present = true;
		/* Fall through */
	case EDDEVID_IMPL_EDPCSR_EDCIDSR:
		drvdata->edcidsr_present = true;
		/* Fall through */
	case EDDEVID_IMPL_EDPCSR:
		/*
		 * In ARM DDI 0487A.k, the EDDEVID1.PCSROffset is used to
		 * define if has the offset for PC sampling value; if read
		 * back EDDEVID1.PCSROffset == 0x2, then this means the debug
		 * module does not sample the instruction set state when
		 * armv8 CPU in AArch32 state.
		 */
		drvdata->edpcsr_present = (IS_ENABLED(CONFIG_64BIT) ||
					   (pcsr_offset != EDDEVID1_PCSR_NO_OFFSET_DIS_AARCH32));
		drvdata->pc_has_offset = (pcsr_offset == EDDEVID1_PCSR_OFFSET_INS_SET);
	default:
		break;
	}

> +	return;
> +}
> +
> +/*
> + * Dump out information on panic.
> + */
> +static int debug_notifier_call(struct notifier_block *self,
> +			       unsigned long v, void *p)
> +{
> +	int cpu;
> +	struct debug_drvdata *drvdata;
> +
> +	pr_emerg("ARM external debug module:\n");
> +
> +	for_each_possible_cpu(cpu) {

Shouldn't this be for_each_online_cpu() ? If the user has turned off a CPU,
we shouldn't try to dump its registers.

> +		drvdata = per_cpu(debug_drvdata, cpu);
> +		if (!drvdata)
> +			continue;
> +
> +		pr_emerg("CPU[%d]:\n", drvdata->cpu);
> +
> +		debug_read_regs(drvdata);
> +		debug_dump_regs(drvdata);
> +	}
> +
> +	return 0;
> +}
> +
> +static struct notifier_block debug_notifier = {
> +	.notifier_call = debug_notifier_call,
> +};
> +
> +static int debug_enable_func(void)
> +{
> +	int ret;
> +

I think we should request the power domain here, now that the user
has explicitly requested the feature to be turned on and released
in debug_disable_func().

> +	pm_qos_add_request(&debug_qos_req,
> +		PM_QOS_CPU_DMA_LATENCY, idle_constraint);
> +
> +	ret = atomic_notifier_chain_register(&panic_notifier_list,
> +					     &debug_notifier);

...

> +	if (ret)
> +		goto err;
> +
> +	return 0;
> +
> +err:
> +	pm_qos_remove_request(&debug_qos_req);
> +	return ret;

nit : this could be :

	if (ret)
		pm_qos_remove_request(&debug_qos_req);
	return ret;

> +}
> +
> +static void debug_disable_func(void)
> +{

As noted above, we could drop the power domain here.

> +	atomic_notifier_chain_unregister(&panic_notifier_list,
> +					 &debug_notifier);
> +	pm_qos_remove_request(&debug_qos_req);
> +}
> +
> +static ssize_t debug_func_knob_write(struct file *f,
> +		const char __user *buf, size_t count, loff_t *ppos)
> +{

> +	if (on) {
> +		ret = debug_enable_func();
> +		if (ret) {
> +			pr_err("%s: unable to disable debug function: %d\n",
> +			       __func__, ret);
> +			goto err;
> +		}
> +	} else
> +		debug_disable_func();

As per Linux codingstyle rules, you should use {} for the else section.
See Documentation/process/coding-style.rst: Section 3.


> +static ssize_t debug_idle_constraint_write(struct file *f,
> +		const char __user *buf, size_t count, loff_t *ppos)
> +{
> +	int val;
> +	ssize_t ret;
> +
> +	ret = kstrtoint_from_user(buf, count, 10, &val);
> +	if (ret)
> +		return ret;
> +
> +	mutex_lock(&debug_lock);
> +	idle_constraint = val;
> +
> +	if (debug_enable)
> +		pm_qos_update_request(&debug_qos_req, idle_constraint);
> +
> +	mutex_unlock(&debug_lock);
> +	return count;
> +}
> +
> +static ssize_t debug_idle_constraint_read(struct file *f,
> +		char __user *ubuf, size_t count, loff_t *ppos)
> +{
> +	char buf[32];
> +	int len;
> +
> +	if (*ppos)
> +		return 0;

It would be better if we do :

> +
> +	len = sprintf(buf, "%d\n", idle_constraint);

	if (*ppos > len)
		return 0;

> +	return simple_read_from_buffer(ubuf, count, ppos, buf, len);

	return simple_read_from_buffer(ubuf, count, ppos, buf + *ppos, len - *ppos);

> +
> +static int debug_func_init(void)
> +{
> +	struct dentry *file;
> +	int ret;

...

> +	/* Enable debug module at boot time */
> +	ret = debug_enable_func();
> +	if (ret) {
> +		pr_err("%s: unable to disable debug function: %d\n",

s/disable/enable ?

> +		       __func__, ret);
> +		goto err;
> +	}
> +
> +	return 0;
> +
> +err:
> +	debugfs_remove_recursive(debug_debugfs_dir);
> +	return ret;
> +}
> +
> +static void debug_func_exit(void)
> +{
> +	debugfs_remove_recursive(debug_debugfs_dir);
> +
> +	/* Disable functionality if has enabled */
> +	if (debug_enable)
> +		debug_disable_func();
> +}
> +
> +static int debug_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> +	void __iomem *base;
> +	struct device *dev = &adev->dev;
> +	struct debug_drvdata *drvdata;
> +	struct resource *res = &adev->res;
> +	struct device_node *np = adev->dev.of_node;
> +	int ret;
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	drvdata->cpu = np ? of_coresight_get_cpu(np) : 0;
> +	if (per_cpu(debug_drvdata, drvdata->cpu)) {
> +		dev_err(dev, "CPU's drvdata has been initialized\n");
> +		return -EBUSY;
> +	}
> +
> +	drvdata->dev = &adev->dev;
> +	amba_set_drvdata(adev, drvdata);
> +
> +	/* Validity for the resource is already checked by the AMBA core */
> +	base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	drvdata->base = base;
> +
> +	get_online_cpus();
> +	per_cpu(debug_drvdata, drvdata->cpu) = drvdata;
> +	ret = smp_call_function_single(drvdata->cpu,
> +				debug_init_arch_data, drvdata, 1);
> +	put_online_cpus();

Now that we have dynamic enable/disable of the feature, we should do a pm_runtime_put()
as expected and try to get_ the power domain when we enable the debug.


Suzuki

^ permalink raw reply

* Re: [PATCH] dt-bindings: bus: TI interconnect target module binding
From: Rob Herring @ 2017-03-27 16:44 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-omap, Benoît Cousson, devicetree@vger.kernel.org,
	Mark Rutland, Nishanth Menon, Matthijs van Duin, Paul Walmsley,
	Peter Ujfalusi, Tero Kristo
In-Reply-To: <20170324033008.GV10760@atomide.com>

On Thu, Mar 23, 2017 at 10:30 PM, Tony Lindgren <tony@atomide.com> wrote:
> Hi,
>
> * Rob Herring <robh@kernel.org> [170323 18:57]:
>> On Tue, Mar 14, 2017 at 02:23:04PM -0700, Tony Lindgren wrote:
>> > +- clocks   shall contain the clocks managed by the interconnect target
>> > +           module, typically at least the clkctrl clock of the module
>>
>> Where do we document exactly how many clocks?
>
> That's the clkctrl clock binding done earlier. Will add a note
> about that.
>
>> > +Example: Single instance of MUSB controller on omap4 using interconnect ranges:
>> > +
>> > +   target_module@2b000 {                   /* 0x4a0ab000, ap 84 12.0 */
>>
>> target-module@
>>
>> This unit address doesn't look right?
>
> Oh OK sure will update.
>
>> > +           compatible = "ti,sysc-type1", "simple-bus";
>>
>> Seems fairly complicated to claim "simple-bus"?
>
> Hmm that allows to update the dts files and keep things working
> with the current "ti,hwmods" property. Then the "ti,hwmods" can be
> just removed to switch over to the new interconnect target driver.
> So both legacy "simple-bus" + "ti,hwmods" will work and new
> "ti,sysc-type1" and no "ti,hwmods". Or do you see some issues with
> that approach?
>
>> > +           #address-cells = <2>;
>>
>> The target module really needs 64-bits of address space?
>
> I guess not :)
>
>> > +           #size-cells = <1>;
>> > +           reg = <0x54 0x400 0x4
>> > +                  0x54 0x404 0x4
>> > +                  0x54 0x408 0x4
>> > +                  0x54 0 0x001000>;
>>
>> You have overlapping regions. Don't do that.
>
> Yeah that can be left out for the interconnect target module if
> we move some of the quirk handling for idling the unused child
> devices into the child device drivers with the earlier proposed
> status = "incomplete" stuff.
>
> That way the interconnect target module does not have to access the
> child IO range. The child will need the 0x54 0x1000 as a range
> though, so it would roughly like this with #address-cells = <1>:
>
>         target-module@2b000 {                   /* 0x4a0ab000, ap 84 12.0 */
>                 compatible = "ti,sysc-type1", "simple-bus";
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>                 reg = <0x54 0x400 0x4
>                        0x54 0x404 0x4
>                        0x54 0x408 0x4>;
>                 reg-names = "rev", "sysc", "syss";
>                 ranges = <0 0x54 0x001000>;
>                 clocks = <&cm_l3init_clkctrl OMAP4_OTG_CLKCTRL 0>;
>                 clock-names = "clkctrl";
>
>                 ti,sysc-has-autoidle;
>                 ti,sysc-has-enawakeup;
>                 ti,sysc-has-softreset;
>                 ...
>
>                 usb_otg_hs: usb_otg_hs@0 {
>                         compatible = "ti,omap4-musb";
>                         reg = <0 0x7ff>;
>                         interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>,
>                                      <GIC_SPI 93 IRQ_TYPE_LEVEL_HIH>;
>                         ...
>                 };
>         };
>
> Note how the ranges provided by the interconnect target module
> still overlaps with the interconnect module control registers.
> Does that look OK to you or do you have some better idea in mind
> for dealing with that?
>
> Splitting the child range into sections would break ioremap for all
> drivers I believe.
>
>> And for the first 3, if you have that much variation, maybe you need
>> more specific compatible strings?
>
> The first three target module regions can be all over the map
> within the child range unfortunately.. See for example just the
> sysc registers below.
>
> Regards,
>
> Tony
>
> 8< ------------
> $ git grep sysc_offs arch/arm/mach-omap2/*hwmod*data*.c | \
>         cut -d'=' -f2 | sort | uniq
>  0x0000,
>  0x0004,
>  0x0010,
>  0x0014,
>  0x002c,
>  0x0034,
>  0x0038,
>  0x0054,
>  0x0078,
>  0x0084,
>  0x008C,
>  0x008c,
>  0x010,
>  0x0110,
>  0x0210,
>  0x0404,
>  0x10,
>  0x104,
>  0x110,
>  0x14,
>  0x1fc10,
>  0x1fe4,
>  0x20,
>  0x24,
>  0x34,
>  0x38,
>  0x4,
>  0x40,
>  0x48,
>  0x54,
>  0x60,
>  0x64,
>  0x78,
>  0x8,
>  0x84,

^ permalink raw reply

* Re: [PATCH 5/5] firmware: google memconsole: Add ARM/ARM64 support
From: Brian Norris @ 2017-03-27 16:56 UTC (permalink / raw)
  To: Thierry Escande
  Cc: Rob Herring, Greg Kroah-Hartman, Olof Johansson, Stephen Warren,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Julius Werner
In-Reply-To: <1490303069-13230-6-git-send-email-thierry.escande-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

Hi Thierry,

In case this wasn't obvious from the build report:

On Thu, Mar 23, 2017 at 10:04:29PM +0100, Thierry Escande wrote:
> --- a/drivers/firmware/google/Kconfig
> +++ b/drivers/firmware/google/Kconfig
> @@ -1,6 +1,5 @@
>  menuconfig GOOGLE_FIRMWARE
>  	bool "Google Firmware Drivers"
> -	depends on X86

The build bots complained: you're relaxing the compile-time restriction
here, so you should probably move it to the GOOGLE_SMI symbol, since
that file uses X86 assembly.

>  	default n
>  	help
>  	  These firmware drivers are used by Google's servers.  They are
> @@ -21,7 +20,7 @@ config GOOGLE_SMI
>  
>  config GOOGLE_COREBOOT_TABLE
>  	tristate
> -	depends on GOOGLE_COREBOOT_TABLE_ACPI
> +	depends on GOOGLE_COREBOOT_TABLE_ACPI || GOOGLE_COREBOOT_TABLE_OF
>  
>  config GOOGLE_COREBOOT_TABLE_ACPI
>  	tristate "Coreboot Table Access - ACPI"
> @@ -33,6 +32,16 @@ config GOOGLE_COREBOOT_TABLE_ACPI
>  	  pointer is accessed through the ACPI "GOOGCB00" object.
>  	  If unsure say N.
>  
> +config GOOGLE_COREBOOT_TABLE_OF
> +	tristate "Coreboot Table Access - Device Tree"
> +	depends on OF
> +	select GOOGLE_COREBOOT_TABLE
> +	help
> +	  This option enable the coreboot_table module, which provide other
> +	  firmware modules to access coreboot table. The coreboot table pointer
> +	  is accessed through the device tree node /firmware/coreboot.
> +	  If unsure say N.
> +
>  config GOOGLE_MEMCONSOLE
>  	tristate
>  	depends on GOOGLE_MEMCONSOLE_X86_LEGACY || GOOGLE_MEMCONSOLE_COREBOOT

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

^ permalink raw reply

* RE: [PATCH v3 4/7] arm: dts: r7s72100: Add pin controller node
From: Chris Brandt @ 2017-03-27 17:12 UTC (permalink / raw)
  To: Jacopo Mondi, geert+renesas@glider.be,
	laurent.pinchart@ideasonboard.com, linus.walleij@linaro.org,
	robh+dt@kernel.org, mark.rutland@arm.com, linux@armlinux.org.uk
  Cc: linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1490368934-12494-5-git-send-email-jacopo+renesas@jmondi.org>

Hi Jacopo,


On Friday, March 24, 2017, Jacopo Mondi
> +	pinctrl: pinctrl@fcfe3000 {
> +		compatible = "renesas,r7s72100-ports";
> +
> +		#pinctrl-cells = <1>;
> +
> +		reg = <0xfcfe3000 0x42C0>;

Out of curiosity, why did you change this from 0x4248 to 0x42C0?

In your update for renesas,rza1-pinctrl.txt, for the 'Example', you changed it from 0x4248 to 0x4230 (which Geert pointed out makes more sense), but then in the actual DT file you changed it to 0x42C0. Typo?


Chris

^ permalink raw reply

* Re: [PATCH] ARM: dts: am335x-evmsk: adjust mmc2 param to allow suspend
From: Tony Lindgren @ 2017-03-27 17:12 UTC (permalink / raw)
  To: Reizer, Eyal
  Cc: Benoît Cousson, Rob Herring,
	linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <8665E2433BC68541A24DFFCA87B70F5B363623DC-1tpBd5JUCm6IQmiDNMet8wC/G2K4zDHf@public.gmane.org>

* Reizer, Eyal <eyalr-l0cyMroinI0@public.gmane.org> [170326 01:55]:
> mmc2 used for wl12xx was missing the keep-power-in suspend
> parameter. As a result the board couldn't reach suspend state.

OK applying into omap-for-v4.11/fixes thanks.

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

^ permalink raw reply

* Re: [PATCH] ARM: dts: am335x-baltos: add LED support
From: Tony Lindgren @ 2017-03-27 17:38 UTC (permalink / raw)
  To: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
  Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <1490368503-4798-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

* yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170324 08:17]:
> From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> 
> All three devices provide GPIO based LEDs named power,
> wlan and app.
> 
> Place the same definition in all three DTS files, as
> there are other devices (not currently upstreamed), that
> use am335x-baltos.dtsi but use different GPIOs for these
> LEDs.

How about just add am335x-baltos-leds.dtsi and include
it for the three boards to avoid duplication?

Regards,

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

^ permalink raw reply

* Re: [PATCH] ARM: dts: am335x-baltos: add LED support
From: Tony Lindgren @ 2017-03-27 17:45 UTC (permalink / raw)
  To: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg
  Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20170327173852.GN10760-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

* Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [170327 10:42]:
> * yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> [170324 08:17]:
> > From: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> > 
> > All three devices provide GPIO based LEDs named power,
> > wlan and app.
> > 
> > Place the same definition in all three DTS files, as
> > there are other devices (not currently upstreamed), that
> > use am335x-baltos.dtsi but use different GPIOs for these
> > LEDs.
> 
> How about just add am335x-baltos-leds.dtsi and include
> it for the three boards to avoid duplication?

Or maybe even am335x-baltos-xyz-devices.dtsi or something if
you have more devices that are shared across some group
of baltos devices.

Regards,

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

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: ata: add DT bindings for ahci-dm816 SATA controller
From: Tejun Heo @ 2017-03-27 18:06 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rob Herring, Mark Rutland, Neil Armstrong, Michael Turquette,
	Kevin Hilman, Patrick Titiano, Tony Lindgren, Sergei Shtylyov,
	linux-ide, devicetree, linux-kernel
In-Reply-To: <1489489491-14195-2-git-send-email-bgolaszewski@baylibre.com>

On Tue, Mar 14, 2017 at 12:04:50PM +0100, Bartosz Golaszewski wrote:
> Add DT bindings for the onboard SATA controller present on the DM816x
> SoCs.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Applied to libata/for-4.12.

Thanks.

-- 
tejun

^ permalink raw reply

* [PATCH] dt-bindings: arm,nvic: Binding for ARM NVIC interrupt controller on Cortex-M
From: Kumar Gala @ 2017-03-27 18:09 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A
  Cc: Kumar Gala, mark.rutland-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Signed-off-by: Kumar Gala <kumar.gala-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Note: This is a new binding and not used by the linux kernel code right now for
the nvic.  The intent would be to support both the current 'arm,armv7m-nvic'
compatible and this binding in the code in the future.  The 'arm,armv7m-nvic'
doesnt have any binding spec covering it today.

 .../bindings/interrupt-controller/arm,nvic.txt     | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/arm,nvic.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,nvic.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,nvic.txt
new file mode 100644
index 0000000..60ee89c
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,nvic.txt
@@ -0,0 +1,37 @@
+* ARM Nested Vector Interrupt Controller (NVIC)
+
+The NVIC provides an interrupt controller that is tightly coupled to
+Cortex-M based processor cores.  The NVIC implemented on different SoCs
+vary in the number of interrupts and priority bits per interrupt.
+
+Main node required properties:
+
+- compatible : should be one of:
+	"arm,v6m-nvic"
+	"arm,v7m-nvic"
+	"arm,v8m-nvic"
+	"arm,nvic"
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+  interrupt source.  The type shall be a <u32> and the value shall be 2.
+
+  The 1st cell contains the interrupt number for the interrupt type.
+
+  The 2nd cell is the priority of the interrupt.
+
+- reg : Specifies base physical address(s) and size of the NVIC registers.
+  This is at a fixed address (0xe000e100) and size (0xc00).
+
+- arm,num-irq-priority-bits: The number of priority bits implemented by the
+  given SoC
+
+Example:
+
+	intc: interrupt-controller@e000e100 {
+		compatible = "arm,nvic";
+		#interrupt-cells = <2>;
+		#address-cells = <1>;
+		interrupt-controller;
+		reg = <0xe000e100 0xc00>;
+		arm,num-irq-priority-bits = <4>;
+	};
-- 
2.9.3

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

^ permalink raw reply related

* Re: [PATCHv3 1/2] mfd: cpcap: Add missing include dependencies
From: Jacek Anaszewski @ 2017-03-27 18:38 UTC (permalink / raw)
  To: Lee Jones
  Cc: Sebastian Reichel, Richard Purdie, Pavel Machek, Tony Lindgren,
	Rob Herring, Mark Rutland, linux-leds, devicetree, linux-kernel
In-Reply-To: <20170327142608.qcobnqrvyu7u4ybz@dell>

On 03/27/2017 04:26 PM, Lee Jones wrote:
> On Fri, 24 Mar 2017, Jacek Anaszewski wrote:
>> On 03/24/2017 09:47 AM, Sebastian Reichel wrote:
>>> This fixes compilation for files, that try to include the
>>> cpcap header in alphabetically sorted #include lists.
>>>
>>> Acked-by: Pavel Machek <pavel@ucw.cz>
>>> Acked-by: Tony Lindgren <tony@atomide.com>
>>> Signed-off-by: Sebastian Reichel <sre@kernel.org>
>>> ---
>>> Changes since PATCHv2:
>>>  - Add Acked-by from Tony & Pavel
>>> ---
>>>  include/linux/mfd/motorola-cpcap.h | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/include/linux/mfd/motorola-cpcap.h b/include/linux/mfd/motorola-cpcap.h
>>> index 793aa695faa0..aefc49cb7ba9 100644
>>> --- a/include/linux/mfd/motorola-cpcap.h
>>> +++ b/include/linux/mfd/motorola-cpcap.h
>>> @@ -14,6 +14,9 @@
>>>   * published by the Free Software Foundation.
>>>   */
>>>  
>>> +#include <linux/device.h>
>>> +#include <linux/regmap.h>
>>> +
>>>  #define CPCAP_VENDOR_ST		0
>>>  #define CPCAP_VENDOR_TI		1
>>
>> Both patches applied to the for-next branch of linux-leds.git, thanks.
> 
> Que?

Ah, sorry, for some reason I silently assumed that Tony gave ack on
your behalf :-) Probably due to lack of your activity in the subject
and Tony's understanding thereof [0].


Can I have your ack for this patch going through LED tree then?


[0] https://lkml.org/lkml/2017/3/23/413

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply

* Re: [PATCH v7 1/2] dt-bindings: Add GPCv2 power gating driver
From: Andrey Smirnov @ 2017-03-27 18:42 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: Mark Rutland, devicetree, linux-kernel, Rob Herring,
	Fabio Estevam, Andrey Yurovsky, Shawn Guo, linux-arm-kernel,
	Lucas Stach
In-Reply-To: <20170324063216.GC12604@b29396-OptiPlex-7040>

On Thu, Mar 23, 2017 at 11:32 PM, Dong Aisheng <dongas86@gmail.com> wrote:
> On Tue, Mar 21, 2017 at 07:50:03AM -0700, Andrey Smirnov wrote:
>> Add DT bindings for power domain driver for GPCv2 IP block found in
>> i.MX7 SoCs.
>>
>> Cc: yurovsky@gmail.com
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> Cc: Dong Aisheng <dongas86@gmail.com>
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  .../devicetree/bindings/power/fsl,imx-gpcv2.txt    | 71 ++++++++++++++++++++++
>>  include/dt-bindings/power/imx7-power.h             | 18 ++++++
>>  2 files changed, 89 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/power/fsl,imx-gpcv2.txt
>>  create mode 100644 include/dt-bindings/power/imx7-power.h
>>
>> diff --git a/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.txt b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.txt
>> new file mode 100644
>> index 0000000..02f45c6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/fsl,imx-gpcv2.txt
>> @@ -0,0 +1,71 @@
>> +Freescale i.MX General Power Controller v2
>> +==========================================
>> +
>> +The i.MX7S/D General Power Control (GPC) block contains Power Gating
>> +Control (PGC) for various power domains.
>> +
>> +Required properties:
>> +
>> +- compatible: Should be "fsl,imx7d-gpc"
>> +
>> +- reg: should be register base and length as documented in the
>> +  datasheet
>> +
>> +- interrupts: Should contain GPC interrupt request 1
>> +
>> +Power domains contained within GPC node are generic power domain
>> +providers, documented in
>> +Documentation/devicetree/bindings/power/power_domain.txt, which are
>> +described as subnodes of the power gating controller 'pgc' node,
>> +which, in turn, is expected to contain the following:
>> +
>> +Required properties:
>> +
>> +- reg: Power domain index. Valid values are defined in
>> +  include/dt-bindings/power/imx7-power.h
>> +
>> +- #power-domain-cells: Should be 0
>> +
>> +Optional properties:
>> +
>> +- power-supply: Power supply used to power the domain
>> +
>> +Example:
>> +
>> +     gpc: gpc@303a0000 {
>> +             compatible = "fsl,imx7d-gpc";
>> +             reg = <0x303a0000 0x1000>;
>> +             interrupt-controller;
>> +             interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
>> +             #interrupt-cells = <3>;
>> +             interrupt-parent = <&intc>;
>> +
>> +             pgc {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +
>> +                     pgc_pcie_phy: power-domain@3 {
>> +                             #power-domain-cells = <0>;
>> +
>> +                             reg = <IMX7_POWER_DOMAIN_PCIE_PHY>;
>> +                             power-supply = <&reg_1p0d>;
>> +                     };
>> +             };
>> +     };
>> +
>> +
>> +Specifying power domain for IP modules
>> +======================================
>> +
>> +IP cores belonging to a power domain should contain a 'power-domains'
>> +property that is a phandle for PGC node representing the domain.
>> +
>> +Example of a device that is part of the PCIE_PHY power domain:
>> +
>> +     pcie: pcie@33800000 {
>> +           reg = <0x33800000 0x4000>,
>> +                 <0x4ff00000 0x80000>;
>> +             /* ... */
>> +             power-domains = <&pgc_pcie_phy>;
>> +             /* ... */
>> +     };
>> diff --git a/include/dt-bindings/power/imx7-power.h b/include/dt-bindings/power/imx7-power.h
>> new file mode 100644
>> index 0000000..eb70023
>> --- /dev/null
>> +++ b/include/dt-bindings/power/imx7-power.h
>> @@ -0,0 +1,18 @@
>> +/*
>> + *  Copyright (C) 2017 Impinj
>> + *
>> + * 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.
>> + */
>> +
>> +#ifndef __DT_BINDINGS_IMX7_POWER_H__
>> +#define __DT_BINDINGS_IMX7_POWER_H__
>> +
>> +#define IMX7_POWER_DOMAIN_USB_HSIC_PHY       0
>> +#define IMX7_POWER_DOMAIN_USB_OTG2_PHY       1
>> +#define IMX7_POWER_DOMAIN_USB_OTG1_PHY       2
>> +#define IMX7_POWER_DOMAIN_PCIE_PHY   3
>> +#define IMX7_POWER_DOMAIN_MIPI_PHY   4
>
> Nitpick: Probably better to define according to the reference manual
> defined order.
>
> 0x800 ~ 0x83F : PGC for A7 core0
> 0x840 ~ 0x87F: PGC for A7 core1
> 0x880 ~ 0x8BF: PGC for A7 SCU
> 0xA00 ~ 0xA3F: PGC for fastmix/megamix
> 0xC00 ~ 0xC3F: PGC for MIPI PHY
> 0xC40 ~ 0xC7F: PGC for PCIE_PHY
> 0xC80 ~ 0xCBF: Reserved
> 0xCC0 ~ 0xCFF: Reserved
> 0xD00 ~ 0xD3F: PGC for USB HSIC PHY
>
> You can drop A7 core/scu/fastmix/megamix as well.
>

Sure, will do.

Thanks,
Andrey Smirnov

^ permalink raw reply


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