Re: Regulator

2022-01-13 Thread Taylor R Campbell
> Date: Thu, 13 Jan 2022 02:26:04 +
> From: Emmanuel Dreyfus 
> 
> Here is acpidrump -dt: https://ftp.espci.fr/shadow/manu/acpidump

This looks like it's `acpidump -t', not `acpidump -dt'?

I looked at the Linux logic, and it appears that in Linux, there is a
nominally bus-independent regulator API which...only does things for
FDT, and just returns dummy regulators that always succeed on ACPI.

Not clear to me this is actually better than #ifdef FDT in any
meaningful way.  In any case, you can just use #ifdef FDT around the
regulator goop for now and we can deal with it another way later.


Re: Regulator

2022-01-12 Thread Emmanuel Dreyfus
On Wed, Jan 12, 2022 at 07:59:27PM +, Taylor R Campbell wrote:
> Can you say more about the platform this is on?  How do you discover
> the touchscreen device -- via what kind of firmware or bus/device
> enumeration?  If this is acpi, can you share acpidump -dt?  Do you
> have documentation for the hardware you can share?

This is a One Netbook A1. An amd64 platform with ACPI.
According to this post, the touchscreen is powered by a Goodix GXTP7386
https://handheld.computer/?page_id=1131

That device is in ACPI DSDT (see below). Taking insights from 
linux/drivers/input/touchscreen/goodix.c I match it using
static const struct device_compatible_entry compat_data[] = {
{ .compat = "GDIX1001" },
{ .compat = "GDIX1002" },
DEVICE_COMPAT_EOL
};

Oddly, that brings me two devices, GXTP7386 and BOSC0200.

Here is acpidrump -dt: https://ftp.espci.fr/shadow/manu/acpidump
Here is GXTP7386 from DSDT:
Device (TPL1)
{Name (HID2, One)
Name (_HID, "GXTP7386")  // _HID: Hardware ID
Name (_CID, "PNP0C50" /* HID Protocol Device (I2C bus) */)  // _CID:
 Compatible ID
Name (_S0W, Zero)  // _S0W: S0 Device Wake State
Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
{
If ((Arg0 == HIDG))
{
Return (HIDD (Arg0, Arg1, Arg2, Arg3, HID2))
}

Return (Buffer (One)
{
 0x00 // .
})
}

Method (_STA, 0, NotSerialized)  // _STA: Status
{
Return (0x0F)
}

Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
{
Name (RBUF, ResourceTemplate ()
{
I2cSerialBusV2 (0x005D, ControllerInitiated, 0x00061A80,
AddressingMode7Bit, "\\_SB.PCI0.I2C2",
0x00, ResourceConsumer, , Exclusive,
)
GpioInt (Level, ActiveLow, Shared, PullDefault, 0x,
"\\_SB.PCI0.GPI0", 0x00, ResourceConsumer, ,
)
{   // Pin list
0x007F
}
})
Return (RBUF) /* \_SB_.PCI0.I2C2.TPL1._CRS.RBUF */
}
}


-- 
Emmanuel Dreyfus
m...@netbsd.org


Re: Regulator

2022-01-12 Thread Taylor R Campbell
> Date: Wed, 12 Jan 2022 14:34:24 +
> From: Emmanuel Dreyfus 
> 
> There is another readblock. The Linux driver for Goodix touchpanel 
> uses a beast called regulator, which seems to control the chip
> power supply:
> ts->avdd28 = devm_regulator_get(dev, "AVDD28");
> (...)
> ts->vddio = devm_regulator_get(dev, "VDDIO");
> (...)
> /* power up the controller */
> error = regulator_enable(ts->avdd28);
> (...)
> error = regulator_enable(ts->vddio);

This is something that will be provided by the fdt, when you are
running it on a platform with fdt.  For example, quoting from
sys/external/gpl2/dist/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi:

touchscreen@5d {
compatible = "goodix,gt917s";
reg = <0x5d>;
interrupt-parent = <&pio>;
interrupts = <7 4 IRQ_TYPE_LEVEL_HIGH>; /* PH4 */
irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
reset-gpios = <&pio 7 11 GPIO_ACTIVE_HIGH>; /* PH11 */
AVDD28-supply = <®_ldo_io0>;
VDDIO-supply = <®_ldo_io0>;
touchscreen-size-x = <720>;
touchscreen-size-y = <1440>;
};

This is part of the fdt bus attachment.  But if the platform isn't
described by fdt, e.g. you're on an acpi system instead, the driver
might not need to mess with regulators at all (or, it might be acpi
with embedded device tree data).

Can you say more about the platform this is on?  How do you discover
the touchscreen device -- via what kind of firmware or bus/device
enumeration?  If this is acpi, can you share acpidump -dt?  Do you
have documentation for the hardware you can share?


Re: Regulator

2022-01-12 Thread Jason Thorpe



> On Jan 12, 2022, at 9:08 AM, Jason Thorpe  wrote:
> 
>> On Jan 12, 2022, at 8:02 AM, Taylor R Campbell 
>>  wrote:
>> 
>>> Date: Wed, 12 Jan 2022 15:42:17 +
>>> From: Robert Swindells 
>>> 
>>> I'm guessing this is a platform that doesn't use FDT.
>>> 
>>> Maybe we need a more general API for regulators.
>> 
>> What other instances would a more general API cover?  Is there some
>> ACPI interface for regulators?
> 
> The ACPI interface for regulators is usually "put device into power state X", 
> but of course that depends on the right ASL being present in the node for the 
> device.

I guess I forgot to mention, it's not necessarily about setting the device into 
a specific power state (like "soft-suspend" vs "deep-sleep" vs "off").  In 
addition to "on" and "off", the regulator API can be used to step voltages.  
This is useful when you want to consume as little power as possible for a given 
performance requirement... like if you're running an interface at a slower 
clock rate (for whatever reason), you can sometimes step down the voltage 
because you don't need as much drive strength to defeat the capacitances of the 
physical device, for example.  I don't know that ACPI really encapsulates that 
(unless there are DSMs or whatever defined for the device type that know how to 
do such stepping).

In general, the philosophy between ACPI and Device Tree are different in this 
regard.  ACPI aims to encapsulate the knowledge in methods that the firmware 
provides, and Device Tree aims to describe the connections between the elements 
and then provides generic interfaces to interact with those elements.

-- thorpej



Re: Regulator

2022-01-12 Thread Jason Thorpe


> On Jan 12, 2022, at 9:17 AM, Jason Thorpe  wrote:
> 
>> Both lima and panfrost need to make calls to the regulator API, as well
>> as clock, reset and interrupt ones.
> 
> Lima will probably be fine because the platforms that need it will use FDT.  
> Except for if you're using that board with ACPI, I guess.  Same situation 
> with resets.  Interrupts are another story and a bit more complicated.

"Resets" is also a little more complicated, because there are two kinds on the 
Device Tree bindings universe:

- Reset Controllers (which use the "Reset" bindings)

- Reset GPIOs (which use the "GPIO" bindings)

Our FDT GPIO and generic GPIO interfaces are in need of harmonizing, and I've 
been thinking about that lately, so hopefully will have some bandwidth to make 
progress on that front in the coming months.

-- thorpej



Re: Regulator

2022-01-12 Thread Taylor R Campbell
> Date: Wed, 12 Jan 2022 09:08:57 -0800
> From: Jason Thorpe 
> 
> Any place where there's an "#ifdef FDT" is a failure to create a
> proper abstraction.

I tend to agree that #if is bad, but just deleting #if without a
better replacement isn't the right thing.  What's the pattern that the
fdt regulators are an instance of, which this should be replaced by?

The only reason #if occurs in linux/regulator/consumer.h is that that
gets included by a header file (nvif/os.h) that is also used on
non-fdt platforms.  The API itself is only used by tegra drm, as far
as I know.

(Could have put the #if in that header file instead, but that would
have made it a local patch which is more trouble for merging.)


Re: Regulator

2022-01-12 Thread Jason Thorpe



> On Jan 12, 2022, at 9:12 AM, Robert Swindells  wrote:
> 
> 
> Taylor R Campbell  wrote:
>> Date: Wed, 12 Jan 2022 15:42:17 +
>>> From: Robert Swindells 
>>> 
>>> I'm guessing this is a platform that doesn't use FDT.
>>> 
>>> Maybe we need a more general API for regulators.
>> 
>> What other instances would a more general API cover?  Is there some
>> ACPI interface for regulators?
> 
> Don't know, somebody could look at what the Linux API covers.
> 
> Could also look at doing clocks, resets and interrupts.

We have one for clocks, but the only way to acquire a clock handle is via FDT 
at the moment.

> Both lima and panfrost need to make calls to the regulator API, as well
> as clock, reset and interrupt ones.

Lima will probably be fine because the platforms that need it will use FDT.  
Except for if you're using that board with ACPI, I guess.  Same situation with 
resets.  Interrupts are another story and a bit more complicated.

-- thorpej



Re: Regulator

2022-01-12 Thread Robert Swindells


Taylor R Campbell  wrote:
> Date: Wed, 12 Jan 2022 15:42:17 +
>> From: Robert Swindells 
>> 
>> I'm guessing this is a platform that doesn't use FDT.
>> 
>> Maybe we need a more general API for regulators.
>
>What other instances would a more general API cover?  Is there some
>ACPI interface for regulators?

Don't know, somebody could look at what the Linux API covers.

Could also look at doing clocks, resets and interrupts.

>> The current situation causes a fair bit of "#ifdef FDT" in the drm
>> compat code.
>
>I count 1:
>
>Am I missing some?

sys/external/bsd/drm2/include/linux/clk.h

I didn't say they were all in the tree right now, I also have them in
interrupt.h and reset.h in my local working copy.

If you want this kind of thing done in a different way then say what it
should be.

>There's also one in linux/clk.h, for clocks, and two in nouveau_pci.c
>and radeon_pci.c, for kicking out the FBT framebuffer; these don't
>appear to be relevant to regulators.

Both lima and panfrost need to make calls to the regulator API, as well
as clock, reset and interrupt ones.


Re: Regulator

2022-01-12 Thread Jason Thorpe



> On Jan 12, 2022, at 9:08 AM, Jason Thorpe  wrote:
> 
> 
> 
>> On Jan 12, 2022, at 8:02 AM, Taylor R Campbell 
>>  wrote:
>> 
>>> Date: Wed, 12 Jan 2022 15:42:17 +
>>> From: Robert Swindells 
>>> 
>>> I'm guessing this is a platform that doesn't use FDT.
>>> 
>>> Maybe we need a more general API for regulators.
>> 
>> What other instances would a more general API cover?  Is there some
>> ACPI interface for regulators?
> 
> The ACPI interface for regulators is usually "put device into power state X", 
> but of course that depends on the right ASL being present in the node for the 
> device.

(Of course I mean AML here, but of course it's all initially written in ASL, 
so...)

> 
>>> The current situation causes a fair bit of "#ifdef FDT" in the drm
>>> compat code.
>> 
>> I count 1:
>> 
>> https://nxr.netbsd.org/xref/src/sys/external/bsd/drm2/include/linux/regulator/consumer.h?r=1.5#39
>> 
>> Am I missing some?
>> 
>> There's also one in linux/clk.h, for clocks, and two in nouveau_pci.c
>> and radeon_pci.c, for kicking out the FBT framebuffer; these don't
>> appear to be relevant to regulators.
> 
> Any place where there's an "#ifdef FDT" is a failure to create a proper 
> abstraction.
> 
> -- thorpej

-- thorpej



Re: Regulator

2022-01-12 Thread Jason Thorpe



> On Jan 12, 2022, at 8:02 AM, Taylor R Campbell 
>  wrote:
> 
>> Date: Wed, 12 Jan 2022 15:42:17 +
>> From: Robert Swindells 
>> 
>> I'm guessing this is a platform that doesn't use FDT.
>> 
>> Maybe we need a more general API for regulators.
> 
> What other instances would a more general API cover?  Is there some
> ACPI interface for regulators?

The ACPI interface for regulators is usually "put device into power state X", 
but of course that depends on the right ASL being present in the node for the 
device.

>> The current situation causes a fair bit of "#ifdef FDT" in the drm
>> compat code.
> 
> I count 1:
> 
> https://nxr.netbsd.org/xref/src/sys/external/bsd/drm2/include/linux/regulator/consumer.h?r=1.5#39
> 
> Am I missing some?
> 
> There's also one in linux/clk.h, for clocks, and two in nouveau_pci.c
> and radeon_pci.c, for kicking out the FBT framebuffer; these don't
> appear to be relevant to regulators.

Any place where there's an "#ifdef FDT" is a failure to create a proper 
abstraction.

-- thorpej



Re: Regulator

2022-01-12 Thread Taylor R Campbell
> Date: Wed, 12 Jan 2022 15:42:17 +
> From: Robert Swindells 
> 
> I'm guessing this is a platform that doesn't use FDT.
> 
> Maybe we need a more general API for regulators.

What other instances would a more general API cover?  Is there some
ACPI interface for regulators?

> The current situation causes a fair bit of "#ifdef FDT" in the drm
> compat code.

I count 1:

https://nxr.netbsd.org/xref/src/sys/external/bsd/drm2/include/linux/regulator/consumer.h?r=1.5#39

Am I missing some?

There's also one in linux/clk.h, for clocks, and two in nouveau_pci.c
and radeon_pci.c, for kicking out the FBT framebuffer; these don't
appear to be relevant to regulators.


Re: Regulator

2022-01-12 Thread Robert Swindells


Jason Thorpe  wrote:
> On Jan 12, 2022, at 6:34 AM, Emmanuel Dreyfus  wrote:
>> 
>> Hello
>> 
>> I am still working on Goodix touchpanel as told in [1] and [2]. I
>> wrote a driver for the Intel GPIO chip that interfaces with Goodix
>> chipreset and interrupt pins. I have not committed yet, because it
>> remains untested.
>> 
>> There is another readblock. The Linux driver for Goodix touchpanel 
>> uses a beast called regulator, which seems to control the chip
>> power supply:
>>ts->avdd28 = devm_regulator_get(dev, "AVDD28");
>> (...)
>>ts->vddio = devm_regulator_get(dev, "VDDIO");
>> (...)
>>/* power up the controller */
>>error = regulator_enable(ts->avdd28);
>> (...)
>>error = regulator_enable(ts->vddio);
>> 
>> Is it another chip that neds another driver? Or a feature of the
>> Goodix chip?  What support do we have for thins kind of thing? 
>> grep -r regulator returns manu hits in src/sys/dev/i2c and
>> src/sys/dev/fdt but we do not have any generic API for that, right?

>We support regulators using FDT.

I'm guessing this is a platform that doesn't use FDT.

Maybe we need a more general API for regulators.

The current situation causes a fair bit of "#ifdef FDT" in the drm
compat code.


Re: Regulator

2022-01-12 Thread Jason Thorpe



> On Jan 12, 2022, at 6:34 AM, Emmanuel Dreyfus  wrote:
> 
> Hello
> 
> I am still working on Goodix touchpanel as told in [1] and [2]. I
> wrote a driver for the Intel GPIO chip that interfaces with Goodix
> chipreset and interrupt pins. I have not committed yet, because it
> remains untested.
> 
> There is another readblock. The Linux driver for Goodix touchpanel 
> uses a beast called regulator, which seems to control the chip
> power supply:
>ts->avdd28 = devm_regulator_get(dev, "AVDD28");
> (...)
>ts->vddio = devm_regulator_get(dev, "VDDIO");
> (...)
>/* power up the controller */
>error = regulator_enable(ts->avdd28);
> (...)
>error = regulator_enable(ts->vddio);
> 
> Is it another chip that neds another driver? Or a feature of the
> Goodix chip?  What support do we have for thins kind of thing? 
> grep -r regulator returns manu hits in src/sys/dev/i2c and
> src/sys/dev/fdt but we do not have any generic API for that, right?

We support regulators using FDT.

-- thorpej



Regulator

2022-01-12 Thread Emmanuel Dreyfus
Hello

I am still working on Goodix touchpanel as told in [1] and [2]. I
wrote a driver for the Intel GPIO chip that interfaces with Goodix
chipreset and interrupt pins. I have not committed yet, because it
remains untested.

There is another readblock. The Linux driver for Goodix touchpanel 
uses a beast called regulator, which seems to control the chip
power supply:
ts->avdd28 = devm_regulator_get(dev, "AVDD28");
(...)
ts->vddio = devm_regulator_get(dev, "VDDIO");
(...)
/* power up the controller */
error = regulator_enable(ts->avdd28);
(...)
error = regulator_enable(ts->vddio);

Is it another chip that neds another driver? Or a feature of the
Goodix chip?  What support do we have for thins kind of thing? 
grep -r regulator returns manu hits in src/sys/dev/i2c and
src/sys/dev/fdt but we do not have any generic API for that, right?

[1] http://mail-index.netbsd.org/tech-kern/2021/12/11/msg027852.html
[2] http://mail-index.netbsd.org/tech-kern/2021/12/18/msg027861.html
-- 
Emmanuel Dreyfus
m...@netbsd.org