Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus
@ 2019-08-02 12:51 Jean Delvare
  2019-08-02 12:52 ` [PATCH v5 1/3] i2c: piix4: Fix port selection for AMD Family 16h Model 30h Jean Delvare
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Jean Delvare @ 2019-08-02 12:51 UTC (permalink / raw)
  To: Linux I2C
  Cc: Wolfram Sang, linux-kernel, Andrew Cooks, linux-acpi,
	platypus-sw, Tobin C . Harding, Guenter Roeck, Will Wagner

These patches fix a couple of issues with the i2c-piix4 driver on
AMD Family 16h Model 30h SoCs and add ACPI-based enumeration to the
i2c-piix4 driver.

Some I2C peripherals, eg. PCA953x IO expander, are not discovered by the
probe or detect mechanisms when attached to an SMBus controller that uses
the i2c-piix4 SMBus driver.

ACPI provides a mechanism to define these peripherals and the controller
port that they're attached to.

Based on earlier work by Andrew Cooks.

Changes:
v5:
  take over from Andrew Cooks who apparently moved to other projects
  fix style issues reported by Tobin C. Harding
  fix potential array overrun
  make sure all registered adapters get unregistered
  keep ports 3 and 4 on early Hudson2
  assume AMD SMBus numbering for ACPI devices
v4:
  remove unnecessary SB800_MAIN_PORTS constant
  reduce piix4_remove change
v3:
  take chip revision into account when determining port selection register
v2:
  count the adapters, instead of misusing port numbers

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 1/3] i2c: piix4: Fix port selection for AMD Family 16h Model 30h
  2019-08-02 12:51 [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus Jean Delvare
@ 2019-08-02 12:52 ` Jean Delvare
  2019-08-02 12:54 ` [PATCH v5 2/3] i2c: piix4: Fix probing of reserved ports on " Jean Delvare
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Jean Delvare @ 2019-08-02 12:52 UTC (permalink / raw)
  To: Linux I2C
  Cc: Wolfram Sang, linux-kernel, Andrew Cooks, linux-acpi,
	platypus-sw, Tobin C . Harding, Guenter Roeck, Will Wagner

From: Andrew Cooks <andrew.cooks@opengear.com>

Family 16h Model 30h SMBus controller needs the same port selection fix
as described and fixed in commit 0fe16195f891 ("i2c: piix4: Fix SMBus port
selection for AMD Family 17h chips")

commit 6befa3fde65f ("i2c: piix4: Support alternative port selection
register") also fixed the port selection for Hudson2, but unfortunately
this is not the exact same device and the AMD naming and PCI Device IDs
aren't particularly helpful here.

The SMBus port selection register is common to the following Families
and models, as documented in AMD's publicly available BIOS and Kernel
Developer Guides:

 50742 - Family 15h Model 60h-6Fh (PCI_DEVICE_ID_AMD_KERNCZ_SMBUS)
 55072 - Family 15h Model 70h-7Fh (PCI_DEVICE_ID_AMD_KERNCZ_SMBUS)
 52740 - Family 16h Model 30h-3Fh (PCI_DEVICE_ID_AMD_HUDSON2_SMBUS)

The Hudson2 PCI Device ID (PCI_DEVICE_ID_AMD_HUDSON2_SMBUS) is shared
between Bolton FCH and Family 16h Model 30h, but the location of the
SmBus0Sel port selection bits are different:

 51192 - Bolton Register Reference Guide

We distinguish between Bolton and Family 16h Model 30h using the PCI
Revision ID:

  Bolton is device 0x780b, revision 0x15
  Family 16h Model 30h is device 0x780b, revision 0x1F
  Family 15h Model 60h and 70h are both device 0x790b, revision 0x4A.

The following additional public AMD BKDG documents were checked and do
not share the same port selection register:

 42301 - Family 15h Model 00h-0Fh doesn't mention any
 42300 - Family 15h Model 10h-1Fh doesn't mention any
 49125 - Family 15h Model 30h-3Fh doesn't mention any

 48751 - Family 16h Model 00h-0Fh uses the previously supported
         index register SB800_PIIX4_PORT_IDX_ALT at 0x2e

Signed-off-by: Andrew Cooks <andrew.cooks@opengear.com>
Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: stable@vger.kernel.org [v4.6+]
---
Changes since v4:
 * Removed unneeded parentheses (reported by Tobin C. Harding)

 drivers/i2c/busses/i2c-piix4.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

--- linux-5.2.orig/drivers/i2c/busses/i2c-piix4.c	2019-07-28 21:57:05.337228192 +0200
+++ linux-5.2/drivers/i2c/busses/i2c-piix4.c	2019-08-02 13:53:33.222597706 +0200
@@ -91,7 +91,7 @@
 #define SB800_PIIX4_PORT_IDX_MASK	0x06
 #define SB800_PIIX4_PORT_IDX_SHIFT	1
 
-/* On kerncz, SmBus0Sel is at bit 20:19 of PMx00 DecodeEn */
+/* On kerncz and Hudson2, SmBus0Sel is at bit 20:19 of PMx00 DecodeEn */
 #define SB800_PIIX4_PORT_IDX_KERNCZ		0x02
 #define SB800_PIIX4_PORT_IDX_MASK_KERNCZ	0x18
 #define SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ	3
@@ -358,18 +358,16 @@ static int piix4_setup_sb800(struct pci_
 	/* Find which register is used for port selection */
 	if (PIIX4_dev->vendor == PCI_VENDOR_ID_AMD ||
 	    PIIX4_dev->vendor == PCI_VENDOR_ID_HYGON) {
-		switch (PIIX4_dev->device) {
-		case PCI_DEVICE_ID_AMD_KERNCZ_SMBUS:
+		if (PIIX4_dev->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS ||
+		    (PIIX4_dev->device == PCI_DEVICE_ID_AMD_HUDSON2_SMBUS &&
+		     PIIX4_dev->revision >= 0x1F)) {
 			piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_KERNCZ;
 			piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK_KERNCZ;
 			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ;
-			break;
-		case PCI_DEVICE_ID_AMD_HUDSON2_SMBUS:
-		default:
+		} else {
 			piix4_port_sel_sb800 = SB800_PIIX4_PORT_IDX_ALT;
 			piix4_port_mask_sb800 = SB800_PIIX4_PORT_IDX_MASK;
 			piix4_port_shift_sb800 = SB800_PIIX4_PORT_IDX_SHIFT;
-			break;
 		}
 	} else {
 		if (!request_muxed_region(SB800_PIIX4_SMB_IDX, 2,


-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 2/3] i2c: piix4: Fix probing of reserved ports on AMD Family 16h Model 30h
  2019-08-02 12:51 [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus Jean Delvare
  2019-08-02 12:52 ` [PATCH v5 1/3] i2c: piix4: Fix port selection for AMD Family 16h Model 30h Jean Delvare
@ 2019-08-02 12:54 ` " Jean Delvare
  2019-08-02 12:55 ` [PATCH v5 3/3] i2c: piix4: Add ACPI support Jean Delvare
  2019-08-08  9:17 ` [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus Enrico Weigelt, metux IT consult
  3 siblings, 0 replies; 10+ messages in thread
From: Jean Delvare @ 2019-08-02 12:54 UTC (permalink / raw)
  To: Linux I2C
  Cc: Wolfram Sang, linux-kernel, Andrew Cooks, linux-acpi,
	platypus-sw, Tobin C . Harding, Guenter Roeck, Will Wagner

Prevent bus timeouts and resets on Family 16h Model 30h by not probing
reserved Ports 3 and 4.

According to the AMD BIOS and Kernel Developer's Guides (BKDG), Port 3
and Port 4 are reserved on the following devices:
 - Family 15h Model 60h-6Fh
 - Family 15h Model 70h-7Fh
 - Family 16h Model 30h-3Fh

Based on earlier work by Andrew Cooks.

Reported-by: Andrew Cooks <andrew.cooks@opengear.com>
Signed-off-by: Jean Delvare <jdelvare@suse.de>
---
Changes since v4:
 * Fix subject line
 * Drop local variable port_count, use piix4_adapter_count
   everywhere to represent the maximum number of main SMBus ports
 * Exclude early Hudson2 implementations from this change

 drivers/i2c/busses/i2c-piix4.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

--- linux-5.2.orig/drivers/i2c/busses/i2c-piix4.c	2019-08-02 14:08:17.003820098 +0200
+++ linux-5.2/drivers/i2c/busses/i2c-piix4.c	2019-08-02 14:15:38.284423549 +0200
@@ -72,7 +72,8 @@
 #define PIIX4_BLOCK_DATA	0x14
 
 /* Multi-port constants */
-#define PIIX4_MAX_ADAPTERS 4
+#define PIIX4_MAX_ADAPTERS	4
+#define HUDSON2_MAIN_PORTS	2 /* HUDSON2, KERNCZ reserves ports 3, 4 */
 
 /* SB800 constants */
 #define SB800_PIIX4_SMB_IDX		0xcd6
@@ -806,6 +807,7 @@ MODULE_DEVICE_TABLE (pci, piix4_ids);
 
 static struct i2c_adapter *piix4_main_adapters[PIIX4_MAX_ADAPTERS];
 static struct i2c_adapter *piix4_aux_adapter;
+static int piix4_adapter_count;
 
 static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
 			     bool sb800_main, u8 port, bool notify_imc,
@@ -865,7 +867,15 @@ static int piix4_add_adapters_sb800(stru
 	int port;
 	int retval;
 
-	for (port = 0; port < PIIX4_MAX_ADAPTERS; port++) {
+	if (dev->device == PCI_DEVICE_ID_AMD_KERNCZ_SMBUS ||
+	    (dev->device == PCI_DEVICE_ID_AMD_HUDSON2_SMBUS &&
+	     dev->revision >= 0x1F)) {
+		piix4_adapter_count = HUDSON2_MAIN_PORTS;
+	} else {
+		piix4_adapter_count = PIIX4_MAX_ADAPTERS;
+	}
+
+	for (port = 0; port < piix4_adapter_count; port++) {
 		retval = piix4_add_adapter(dev, smba, true, port, notify_imc,
 					   piix4_main_port_names_sb800[port],
 					   &piix4_main_adapters[port]);
@@ -987,7 +997,7 @@ static void piix4_adap_remove(struct i2c
 
 static void piix4_remove(struct pci_dev *dev)
 {
-	int port = PIIX4_MAX_ADAPTERS;
+	int port = piix4_adapter_count;
 
 	while (--port >= 0) {
 		if (piix4_main_adapters[port]) {

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 3/3] i2c: piix4: Add ACPI support
  2019-08-02 12:51 [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus Jean Delvare
  2019-08-02 12:52 ` [PATCH v5 1/3] i2c: piix4: Fix port selection for AMD Family 16h Model 30h Jean Delvare
  2019-08-02 12:54 ` [PATCH v5 2/3] i2c: piix4: Fix probing of reserved ports on " Jean Delvare
@ 2019-08-02 12:55 ` Jean Delvare
  2019-08-08  9:17 ` [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus Enrico Weigelt, metux IT consult
  3 siblings, 0 replies; 10+ messages in thread
From: Jean Delvare @ 2019-08-02 12:55 UTC (permalink / raw)
  To: Linux I2C
  Cc: Wolfram Sang, linux-kernel, Andrew Cooks, linux-acpi,
	platypus-sw, Tobin C . Harding, Guenter Roeck, Will Wagner

Enable the i2c-piix4 SMBus controller driver to enumerate I2C slave
devices using ACPI. It builds on the related I2C mux device work
in commit 8eb5c87a92c0 ("i2c: add ACPI support for I2C mux ports")

In the i2c-piix4 driver the adapters are enumerated as:
 Main SMBus adapter Port 0, Port 2, ..., aux port (i.e., ASF adapter)

However, in the AMD BKDG documentation[1], the implied order of ports is:
 Main SMBus adapter Port 0, ASF adapter, Port 2, Port 3, ...

This ordering difference is unfortunate. We assume that ACPI
developers will use the AMD documentation ordering, so we have to
pass an extra parameter to piix4_add_adapter().

[1] 52740 BIOS and Kernel Developer's Guide (BKDG) for AMD Family 16h
Models 30h-3Fh Processors

Based on earlier work by Andrew Cooks.

Reported-by: Andrew Cooks <andrew.cooks@opengear.com>
Signed-off-by: Jean Delvare <jdelvare@suse.de>
---
Changes since v4:
 * Fix code alignment (reported by Tobin C. Harding)
 * Adjust to changes in previous patch
 * Use the SMBus numbering documented by AMD

 drivers/i2c/busses/i2c-piix4.c |   18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

--- linux-5.2.orig/drivers/i2c/busses/i2c-piix4.c	2019-08-02 14:26:02.197346075 +0200
+++ linux-5.2/drivers/i2c/busses/i2c-piix4.c	2019-08-02 14:40:40.990505068 +0200
@@ -811,7 +811,8 @@ static int piix4_adapter_count;
 
 static int piix4_add_adapter(struct pci_dev *dev, unsigned short smba,
 			     bool sb800_main, u8 port, bool notify_imc,
-			     const char *name, struct i2c_adapter **padap)
+			     u8 hw_port_nr, const char *name,
+			     struct i2c_adapter **padap)
 {
 	struct i2c_adapter *adap;
 	struct i2c_piix4_adapdata *adapdata;
@@ -843,6 +844,12 @@ static int piix4_add_adapter(struct pci_
 	/* set up the sysfs linkage to our parent device */
 	adap->dev.parent = &dev->dev;
 
+	if (has_acpi_companion(&dev->dev)) {
+		acpi_preset_companion(&adap->dev,
+				      ACPI_COMPANION(&dev->dev),
+				      hw_port_nr);
+	}
+
 	snprintf(adap->name, sizeof(adap->name),
 		"SMBus PIIX4 adapter%s at %04x", name, smba);
 
@@ -876,7 +883,10 @@ static int piix4_add_adapters_sb800(stru
 	}
 
 	for (port = 0; port < piix4_adapter_count; port++) {
+		u8 hw_port_nr = port == 0 ? 0 : port + 1;
+
 		retval = piix4_add_adapter(dev, smba, true, port, notify_imc,
+					   hw_port_nr,
 					   piix4_main_port_names_sb800[port],
 					   &piix4_main_adapters[port]);
 		if (retval < 0)
@@ -947,8 +957,8 @@ static int piix4_probe(struct pci_dev *d
 			return retval;
 
 		/* Try to register main SMBus adapter, give up if we can't */
-		retval = piix4_add_adapter(dev, retval, false, 0, false, "",
-					   &piix4_main_adapters[0]);
+		retval = piix4_add_adapter(dev, retval, false, 0, false, 0,
+					   "", &piix4_main_adapters[0]);
 		if (retval < 0)
 			return retval;
 	}
@@ -974,7 +984,7 @@ static int piix4_probe(struct pci_dev *d
 	if (retval > 0) {
 		/* Try to add the aux adapter if it exists,
 		 * piix4_add_adapter will clean up if this fails */
-		piix4_add_adapter(dev, retval, false, 0, false,
+		piix4_add_adapter(dev, retval, false, 0, false, 1,
 				  is_sb800 ? piix4_aux_port_name_sb800 : "",
 				  &piix4_aux_adapter);
 	}

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus
  2019-08-02 12:51 [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus Jean Delvare
                   ` (2 preceding siblings ...)
  2019-08-02 12:55 ` [PATCH v5 3/3] i2c: piix4: Add ACPI support Jean Delvare
@ 2019-08-08  9:17 ` Enrico Weigelt, metux IT consult
  2019-08-09  8:33   ` Jean Delvare
  2019-08-11  2:52   ` Andrew Cooks
  3 siblings, 2 replies; 10+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2019-08-08  9:17 UTC (permalink / raw)
  To: Jean Delvare, Linux I2C
  Cc: Wolfram Sang, linux-kernel, Andrew Cooks, linux-acpi,
	platypus-sw, Tobin C . Harding, Guenter Roeck, Will Wagner

On 02.08.19 14:51, Jean Delvare wrote:

Hi,

> These patches fix a couple of issues with the i2c-piix4 driver on
> AMD Family 16h Model 30h SoCs and add ACPI-based enumeration to the
> i2c-piix4 driver.

Can you tell a little bit more about what devices are behind the smbus ?
I recall the G-412 SoCs (such as on apu2+ boards) have an Hudson inside
and fall into this category. (I'll have to check when back in office),
so (as the apu2 platform driver maintainer) I'm very interested in this.

Does the probing need some special BIOS support (or do the necessary
table entries already come from aegesa) ?

I have to admit, I'm still confused by the AMD documentation - haven't
found a clear documentation on what peripherals exactly are in the
G-412 SoC, just puzzled together that the FCH seems to be an Hudson,
probably v2. There also seems to be some relation between smbus and
gpio, but the gpio's are directly memory-mapped - no idea whether they
just share the same base address register or the gpios are really behind
smbus and some hw logic directy maps them into mmio space ...
Do you happen to have some more information on that ?

By the way: I'm considering collecting some hw documentation in the
kernel tree (maybe Documentation/hardware/...) - do you folks think
that's a good idea ?

--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus
  2019-08-08  9:17 ` [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus Enrico Weigelt, metux IT consult
@ 2019-08-09  8:33   ` Jean Delvare
  2019-08-09 15:53     ` Enrico Weigelt, metux IT consult
  2019-08-11  3:09     ` Andrew Cooks
  2019-08-11  2:52   ` Andrew Cooks
  1 sibling, 2 replies; 10+ messages in thread
From: Jean Delvare @ 2019-08-09  8:33 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Linux I2C, Wolfram Sang, linux-kernel, Andrew Cooks, linux-acpi,
	platypus-sw, Tobin C . Harding, Guenter Roeck, Will Wagner

Hi Enrico,

On Thu, 8 Aug 2019 11:17:53 +0200, Enrico Weigelt, metux IT consult wrote:
> On 02.08.19 14:51, Jean Delvare wrote:
> > These patches fix a couple of issues with the i2c-piix4 driver on
> > AMD Family 16h Model 30h SoCs and add ACPI-based enumeration to the
> > i2c-piix4 driver.  
> 
> Can you tell a little bit more about what devices are behind the smbus ?
> I recall the G-412 SoCs (such as on apu2+ boards) have an Hudson inside
> and fall into this category. (I'll have to check when back in office),
> so (as the apu2 platform driver maintainer) I'm very interested in this.

Unfortunately not. I only picked up from where Andrew Cooks left, due
to me being way too slow to review his patches. I did not want his work
to be lost. I was able to test the first 2 patches which fix bugs, but
not the 3rd one which deals with ACPI devices. There does not seem to
be any such device on the 2 test machines I have remotely access to.

> Does the probing need some special BIOS support (or do the necessary
> table entries already come from aegesa) ?

I assume that ACPI devices are declared in one of the ACPI tables, so
it comes from the "BIOS", yes, whatever form it takes these days.

> I have to admit, I'm still confused by the AMD documentation - haven't
> found a clear documentation on what peripherals exactly are in the
> G-412 SoC, just puzzled together that the FCH seems to be an Hudson,
> probably v2. There also seems to be some relation between smbus and
> gpio, but the gpio's are directly memory-mapped - no idea whether they
> just share the same base address register or the gpios are really behind
> smbus and some hw logic directy maps them into mmio space ...
> Do you happen to have some more information on that ?

I remember noticing long ago that SMBus ports were using GPIO pins, so
these pins could be used for SMBus or for any other purpose. I could
not find any way to figure out from the registers if a given pin pair
was used for SMBus or not, which is pretty bad because it means we are
blindly instantiating ALL possible SMBus ports even if some of the pins
are used for a completely different purpose. It was over 1 year ago
though, so I don't remember the details, and my findings then may not
apply to the most recent hardware.

> By the way: I'm considering collecting some hw documentation in the
> kernel tree (maybe Documentation/hardware/...) - do you folks think
> that's a good idea ?

No. Only documentation specifically related to the Linux kernel should
live in the kernel tree. OS-neutral documentation must go somewhere
else.

-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus
  2019-08-09  8:33   ` Jean Delvare
@ 2019-08-09 15:53     ` Enrico Weigelt, metux IT consult
  2019-08-14 16:07       ` Wolfram Sang
  2019-08-11  3:09     ` Andrew Cooks
  1 sibling, 1 reply; 10+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2019-08-09 15:53 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Linux I2C, Wolfram Sang, linux-kernel, Andrew Cooks, linux-acpi,
	platypus-sw, Tobin C . Harding, Guenter Roeck, Will Wagner

On 09.08.19 10:33, Jean Delvare wrote:

Hi,

> Unfortunately not. I only picked up from where Andrew Cooks left, due
> to me being way too slow to review his patches. 

@Andrew: can you tell us more about this ?

> I was able to test the first 2 patches which fix bugs, but
> not the 3rd one which deals with ACPI devices. There does not seem to
> be any such device on the 2 test machines I have remotely access to.

Did you already test on apu2/apu3 ?
If not, maybe you could prepare a queue that I could test.

>> Does the probing need some special BIOS support (or do the necessary
>> table entries already come from aegesa) ?
> 
> I assume that ACPI devices are declared in one of the ACPI tables, so
> it comes from the "BIOS", yes, whatever form it takes these days.

hmm, so we'll yet have to find out whether these entries are actually
present on actual machines in the field and potentially stick w/
board specific platform drivers.

I had hoped I could do all the probing of things like gpio, etc, from
firmware (grmpf, w/ oftree all of that would be pretty trivial :o),
but I doubt that it will work. Even w/ fairly recent gpio support in
ACPI (IIRC my apu's dont have this yet), we're still lacking the
actual assignment of the gpios (LEDs, Keys, ...).

> I remember noticing long ago that SMBus ports were using GPIO pins, so
> these pins could be used for SMBus or for any other purpose. 

You mean via bit banging ? Or smbus and gpio shared behind a pinmux ?

That might explain the strange holes in the register set (actually,
never tried using anything undocumented as gpio).

Did you find some documents you could send over ?

> I could
> not find any way to figure out from the registers if a given pin pair
> was used for SMBus or not, which is pretty bad because it means we are
> blindly instantiating ALL possible SMBus ports even if some of the pins
> are used for a completely different purpose. 

Do you know the addresses of the smbus port registers ?

These are the gpio registers I've found out - relative to fch base
(0xFED80000) plus gpio offset (0x1500):

/*
  * gpio register index definitions
  */
#define AMD_FCH_GPIO_REG_GPIO49         0x40
#define AMD_FCH_GPIO_REG_GPIO50         0x41
#define AMD_FCH_GPIO_REG_GPIO51         0x42
#define AMD_FCH_GPIO_REG_GPIO59_DEVSLP0 0x43
#define AMD_FCH_GPIO_REG_GPIO57         0x44
#define AMD_FCH_GPIO_REG_GPIO58         0x45
#define AMD_FCH_GPIO_REG_GPIO59_DEVSLP1 0x46
#define AMD_FCH_GPIO_REG_GPIO64         0x47
#define AMD_FCH_GPIO_REG_GPIO68         0x48
#define AMD_FCH_GPIO_REG_GPIO66_SPKR    0x5B
#define AMD_FCH_GPIO_REG_GPIO71         0x4D
#define AMD_FCH_GPIO_REG_GPIO32_GE1     0x59
#define AMD_FCH_GPIO_REG_GPIO33_GE2     0x5A
#define AMT_FCH_GPIO_REG_GEVT22         0x09

(see: include/linux/platform_data/gpio/gpio-amd-fch.h)

>> By the way: I'm considering collecting some hw documentation in the
>> kernel tree (maybe Documentation/hardware/...) - do you folks think
>> that's a good idea ?
> 
> No. Only documentation specifically related to the Linux kernel should
> live in the kernel tree. OS-neutral documentation must go somewhere
> else.

hmm, but dts is also kinda documentation, isn't it ? ;-)

Well, I'll probably start a separate project for that.


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus
  2019-08-08  9:17 ` [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus Enrico Weigelt, metux IT consult
  2019-08-09  8:33   ` Jean Delvare
@ 2019-08-11  2:52   ` Andrew Cooks
  1 sibling, 0 replies; 10+ messages in thread
From: Andrew Cooks @ 2019-08-11  2:52 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, Jean Delvare, Linux I2C
  Cc: Wolfram Sang, linux-kernel, linux-acpi, platypus-sw,
	Tobin C . Harding, Guenter Roeck, Will Wagner

Hi Enrico

On 8/8/19 7:17 PM, Enrico Weigelt, metux IT consult wrote:
> On 02.08.19 14:51, Jean Delvare wrote:
>
> Hi,
>
>> These patches fix a couple of issues with the i2c-piix4 driver on
>> AMD Family 16h Model 30h SoCs and add ACPI-based enumeration to the
>> i2c-piix4 driver.
> Can you tell a little bit more about what devices are behind the smbus ?
> I recall the G-412 SoCs (such as on apu2+ boards) have an Hudson inside
> and fall into this category. (I'll have to check when back in office),
> so (as the apu2 platform driver maintainer) I'm very interested in this.
My initial work is based on a board that is similar to the APU2, but has additional peripherals connected to the smbus, including a NCT7491 thermal monitor/fan controller and PCA6524 GPIO controller. These are simply peripherals on a board variant, not 'platform' devices, so I didn't want to follow the platform driver approach that the APU2 GPIO driver uses.
>
> Does the probing need some special BIOS support (or do the necessary
> table entries already come from aegesa) ?

SMBus (and I2C) peripherals can generally not be enumerated without some firmware support. It is possible to probe for specific devices on the bus (eg sensors-detect) but in general it is not feasible to let every supported device driver probe the bus for its device. ACPI and Devicetree provides the kernel with metadata for the device: type, address, calibrated set points for temperature, etc.

Since the peripherals are not standard platform devices, they are not described by the ACPI tables provided by Coreboot or AMD, but it's not too difficult to create supplementary device description tables (ACPI) for non-standard devices. These can be added to coreboot, supplied to qemu as additional firmware files (see -acpitable arg), or built into the kernel (see https://www.kernel.org/doc/Documentation/acpi/ssdt-overlays.txt)

ACPI may be an ugly abomination, but it's what we're stuck with on x86 and it can only improve when more people get their hands on it.

>
> I have to admit, I'm still confused by the AMD documentation - haven't
> found a clear documentation on what peripherals exactly are in the
> G-412 SoC, just puzzled together that the FCH seems to be an Hudson,
> probably v2. There also seems to be some relation between smbus and
> gpio, but the gpio's are directly memory-mapped - no idea whether they
> just share the same base address register or the gpios are really behind
> smbus and some hw logic directy maps them into mmio space ...
> Do you happen to have some more information on that ?
You might find it helpful to look at the coreboot source for the APU2 (src/mainboard/pcengines/apu2/gpio_ftns.h)
>
> By the way: I'm considering collecting some hw documentation in the
> kernel tree (maybe Documentation/hardware/...) - do you folks think
> that's a good idea ?

Would it be awesome if specs were available for every device supported by the kernel? Absolutely, what a dream! Perhaps a separate repo, like the linux-firmware repo, would be better for binary objects that don't change. Unfortunately, copyright makes this hard. NDAs make this hard. Hardware companies just don't seem to work like that.

>
> --mtx

Regards,
 

Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus
  2019-08-09  8:33   ` Jean Delvare
  2019-08-09 15:53     ` Enrico Weigelt, metux IT consult
@ 2019-08-11  3:09     ` Andrew Cooks
  1 sibling, 0 replies; 10+ messages in thread
From: Andrew Cooks @ 2019-08-11  3:09 UTC (permalink / raw)
  To: Jean Delvare, Enrico Weigelt, metux IT consult
  Cc: Linux I2C, Wolfram Sang, linux-kernel, linux-acpi, platypus-sw,
	Tobin C . Harding, Guenter Roeck, Will Wagner


On 8/9/19 6:33 PM, Jean Delvare wrote:
> Hi Enrico,
>
> On Thu, 8 Aug 2019 11:17:53 +0200, Enrico Weigelt, metux IT consult wrote:
>> On 02.08.19 14:51, Jean Delvare wrote:
>>> These patches fix a couple of issues with the i2c-piix4 driver on
>>> AMD Family 16h Model 30h SoCs and add ACPI-based enumeration to the
>>> i2c-piix4 driver.  
>> Can you tell a little bit more about what devices are behind the smbus ?
>> I recall the G-412 SoCs (such as on apu2+ boards) have an Hudson inside
>> and fall into this category. (I'll have to check when back in office),
>> so (as the apu2 platform driver maintainer) I'm very interested in this.
> Unfortunately not. I only picked up from where Andrew Cooks left, due
> to me being way too slow to review his patches. I did not want his work
> to be lost. I was able to test the first 2 patches which fix bugs, but
> not the 3rd one which deals with ACPI devices. There does not seem to
> be any such device on the 2 test machines I have remotely access to.

Thanks for taking a look at these patches and thanks for your many years of support and maintenance.

The patches I submitted were developed for an commercial product, but I am not with the company anymore and do not have access to the hardware. This is the device: https://opengear.com/products/om2200-operations-manager/

The specific peripheral that required ACPI support is the NCT7491, and a driver is available at https://github.com/opengear/nct7491-driver

>> Does the probing need some special BIOS support (or do the necessary
>> table entries already come from aegesa) ?
> I assume that ACPI devices are declared in one of the ACPI tables, so
> it comes from the "BIOS", yes, whatever form it takes these days.
Yes, though unfortunately I didn't get a chance to submit this to the coreboot project and no longer have access to the source.
>
>> I have to admit, I'm still confused by the AMD documentation - haven't
>> found a clear documentation on what peripherals exactly are in the
>> G-412 SoC, just puzzled together that the FCH seems to be an Hudson,
>> probably v2. There also seems to be some relation between smbus and
>> gpio, but the gpio's are directly memory-mapped - no idea whether they
>> just share the same base address register or the gpios are really behind
>> smbus and some hw logic directy maps them into mmio space ...
>> Do you happen to have some more information on that ?
> I remember noticing long ago that SMBus ports were using GPIO pins, so
> these pins could be used for SMBus or for any other purpose. I could
> not find any way to figure out from the registers if a given pin pair
> was used for SMBus or not, which is pretty bad because it means we are
> blindly instantiating ALL possible SMBus ports even if some of the pins
> are used for a completely different purpose. It was over 1 year ago
> though, so I don't remember the details, and my findings then may not
> apply to the most recent hardware.
>
>> By the way: I'm considering collecting some hw documentation in the
>> kernel tree (maybe Documentation/hardware/...) - do you folks think
>> that's a good idea ?
> No. Only documentation specifically related to the Linux kernel should
> live in the kernel tree. OS-neutral documentation must go somewhere
> else.
>
Thanks,

Andrew


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus
  2019-08-09 15:53     ` Enrico Weigelt, metux IT consult
@ 2019-08-14 16:07       ` Wolfram Sang
  0 siblings, 0 replies; 10+ messages in thread
From: Wolfram Sang @ 2019-08-14 16:07 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Jean Delvare, Linux I2C, linux-kernel, Andrew Cooks, linux-acpi,
	platypus-sw, Tobin C . Harding, Guenter Roeck, Will Wagner

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


> > Unfortunately not. I only picked up from where Andrew Cooks left, due
> > to me being way too slow to review his patches.
> 
> @Andrew: can you tell us more about this ?

Was the info Andrew supplied helpful to you?

> 
> > I was able to test the first 2 patches which fix bugs, but
> > not the 3rd one which deals with ACPI devices. There does not seem to
> > be any such device on the 2 test machines I have remotely access to.
> 
> Did you already test on apu2/apu3 ?
> If not, maybe you could prepare a queue that I could test.

Maybe I am missing something but can't you just apply these patches on a
recent kernel?

So, the discussion stalled, but I think the series is fine as is? If
someone disagrees, please speak up. I want to apply patch 1 to
for-current soon and the others to for-next, too.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-02 12:51 [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus Jean Delvare
2019-08-02 12:52 ` [PATCH v5 1/3] i2c: piix4: Fix port selection for AMD Family 16h Model 30h Jean Delvare
2019-08-02 12:54 ` [PATCH v5 2/3] i2c: piix4: Fix probing of reserved ports on " Jean Delvare
2019-08-02 12:55 ` [PATCH v5 3/3] i2c: piix4: Add ACPI support Jean Delvare
2019-08-08  9:17 ` [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus Enrico Weigelt, metux IT consult
2019-08-09  8:33   ` Jean Delvare
2019-08-09 15:53     ` Enrico Weigelt, metux IT consult
2019-08-14 16:07       ` Wolfram Sang
2019-08-11  3:09     ` Andrew Cooks
2019-08-11  2:52   ` Andrew Cooks

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org linux-acpi@archiver.kernel.org
	public-inbox-index linux-acpi


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/ public-inbox