Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-28 Thread Andrew Cooks


On 8/20/19 4:30 AM, Enrico Weigelt, metux IT consult wrote:
> On 11.08.19 04:52, Andrew Cooks wrote:
>
> Hi,
>
>> 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.
> Why are they not platform devices ?
>
> IMHO, a platform device is something not on a bus that can do the
> probing/enumeration (like eg. pci or usb does).
>
> Can these devices be probed directly by the bus they're connected to,
> or do you have a different definition of the term "platform device" ?

ACPI was created to provide the PNP mechanism for buses that do not support 
enumeration, like SMBus. SMBus is similar to I2C, and can be routed to 
expansion cards, like Ethernet NICs. A system with expansion card Foo is not a 
different platform from a system with expansion card Bar, and you wouldn't 
necessarily want to support both Foo and Bar in the same platform driver.

>
> :O
>
>> 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.
> Yes, I know. My question was whether these particular devices - if
> they're in the SoC itself - do need some extra support in BIOS
> (acpi entries, etc), that's not already in aegesa. Or in other words:
> do I need to patch my BIOS ?
>
> I was assuming these devices are in the SoC itself - correct me if
> I'm wrong.

This patch is about adding a mechanism to describe peripherals that are not in 
the SoC, but attached to the SMBus. I also provided examples of the kinds of 
devices that need this support (eg. NCT7491).

>
>> 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)
> Oh, I didn't know about SSDT overlays yet. That's interesting.
>
> Is it also possible to describe things like leds-gpio or gpio-keyboard ?
> Could be a nice idea to trim down my apu2 driver to not much more than
> a piece of configuration (just like we'd do on a DT-based system)
Yes and maybe. The extra ACPI layer is inconvenient and more complex than a 
platform driver, as should be evident from the patch history and this 
discussion.
>
>> 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.
> hmm, I'm planning to introduce DT to x86 (also patch up apu'2 coreboot
> do deliver a DT to the kernel) ... but just lacking time to get my hands
> dirty on this topic.
>
>> You might find it helpful to look at the coreboot source for the APU2 
>> (src/mainboard/pcengines/apu2/gpio_ftns.h)
> In which version ?
>
>
> --mtx
>


Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-20 Thread Wolfram Sang

> Yes, I'm struggling to find out which devices are connected to the
> smbus, in case there're some already inside the SoC (instead of just
> entirely board specific).

I see.

> 
> > but you are okay with Jean's patches? Or is this
> > discussion affecting patch 3? (/me knows not much about ACPI'n'stuff)
> 
> I'm fine with them, just wondered whether BIOS needs to add some extra
> support for the probing, that is not already somehow coming w/ aegesa.

Ok, thanks for the heads up.

> Unfortunately, we cannot rely on board vendors to do that right and
> cope w/ old and incomplete firmware. (that's also the reason why I'm
> *not* using acpi gpio entries for the apu2+ board family).

Guess we all have been there :/ Good luck!



signature.asc
Description: PGP signature


Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-20 Thread Enrico Weigelt, metux IT consult

On 19.08.19 20:53, Wolfram Sang wrote:


Just so I get this correct: This is all about instantiating the devices
sitting on the SMBus,


Yes, I'm struggling to find out which devices are connected to the
smbus, in case there're some already inside the SoC (instead of just
entirely board specific).


but you are okay with Jean's patches? Or is this
discussion affecting patch 3? (/me knows not much about ACPI'n'stuff)


I'm fine with them, just wondered whether BIOS needs to add some extra
support for the probing, that is not already somehow coming w/ aegesa.

Unfortunately, we cannot rely on board vendors to do that right and
cope w/ old and incomplete firmware. (that's also the reason why I'm
*not* using acpi gpio entries for the apu2+ board family).


--mtx

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


Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-19 Thread Wolfram Sang

Just so I get this correct: This is all about instantiating the devices
sitting on the SMBus, but you are okay with Jean's patches? Or is this
discussion affecting patch 3? (/me knows not much about ACPI'n'stuff)



signature.asc
Description: PGP signature


Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-19 Thread Enrico Weigelt, metux IT consult

On 11.08.19 04:52, Andrew Cooks wrote:

Hi,


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.


Why are they not platform devices ?

IMHO, a platform device is something not on a bus that can do the
probing/enumeration (like eg. pci or usb does).

Can these devices be probed directly by the bus they're connected to,
or do you have a different definition of the term "platform device" ?

:O


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.


Yes, I know. My question was whether these particular devices - if
they're in the SoC itself - do need some extra support in BIOS
(acpi entries, etc), that's not already in aegesa. Or in other words:
do I need to patch my BIOS ?

I was assuming these devices are in the SoC itself - correct me if
I'm wrong.


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)


Oh, I didn't know about SSDT overlays yet. That's interesting.

Is it also possible to describe things like leds-gpio or gpio-keyboard ?
Could be a nice idea to trim down my apu2 driver to not much more than
a piece of configuration (just like we'd do on a DT-based system)


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.


hmm, I'm planning to introduce DT to x86 (also patch up apu'2 coreboot
do deliver a DT to the kernel) ... but just lacking time to get my hands
dirty on this topic.


You might find it helpful to look at the coreboot source for the APU2 
(src/mainboard/pcengines/apu2/gpio_ftns.h)


In which version ?


--mtx

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


Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-14 Thread Wolfram Sang

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



signature.asc
Description: PGP signature


Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-10 Thread Andrew Cooks
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


Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-10 Thread Andrew Cooks


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



Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-09 Thread Enrico Weigelt, metux IT consult

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
(0xFED8) 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_SPKR0x5B
#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
i...@metux.net -- +49-151-27565287


Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-09 Thread Jean Delvare
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


Re: [PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-08 Thread Enrico Weigelt, metux IT consult

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
i...@metux.net -- +49-151-27565287


[PATCH v5 0/3] Enable ACPI-defined peripherals on i2c-piix4 SMBus

2019-08-02 Thread Jean Delvare
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