On Fri, Jul 30, 2021 at 09:57:04PM +0200, Wolfram Sang wrote:
...
> > 'For ACPI one may use PRP0001 approach with the following ASL excerpt example::
> >
> > Device (GSLA) {
> > Name (_HID, "PRP0001")
> > Name (_DDN, "GPIO sloppy logic analyzer")
> > Name (_CRS, ResourceTemplate () {
> > GpioIo(Exclusive, PullNone, 0, 0, IoRestrictionNone,
> > "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 13 }
> > PinConfig(Exclusive, 0x07, 0, "\\_SB.PCI0.GPIO", 0, ResourceConsumer, ) { 7 }
> > GpioIo(Exclusive, PullNone, 0, 0, IoRestrictionNone,
> > "\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 12 }
> > PinConfig(Exclusive, 0x07, 0, "\\_SB.PCI0.GPIO", 0, ResourceConsumer, ) { 6 }
> > })
> >
> > Name (_DSD, Package () {
> > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > Package () {
> > Package () { "compatible", Package () { "gpio-sloppy-logic-analyzer" } },
> > Package () {
> > "probe-gpios", Package () {
> > ^GSLA, 0, 0, 0,
> > ^GSLA, 1, 0, 0,
> > },
> > Package () {
> > "probe-names", Package () {
> > "SCL",
> > "SDA",
> > },
> > }
> > })
> >
> > Note, that pin configuration uses pin numbering space, while GPIO resources
> > are in GPIO numbering space, which may be different in ACPI. In other words,
> > there is no guarantee that GPIO and pins are mapped 1:1, that's why there are
> > two different pairs in the example, i.e. {13,12} GPIO vs. {7,6} pin.
> >
> > Yet pin configuration support in Linux kernel is subject to implement.'
>
> Have you tested this snippet?
Nope. Below is the compile-tested one:
Device (GSLA) {
Name (_HID, "PRP0001")
Name (_DDN, "GPIO sloppy logic analyzer")
Name (_CRS, ResourceTemplate () {
GpioIo(Exclusive, PullNone, 0, 0, IoRestrictionNone,
"\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 13 }
PinConfig(Exclusive, 0x07, 0, "\\_SB.PCI0.GPIO", 0, ResourceConsumer, ) { 7 }
GpioIo(Exclusive, PullNone, 0, 0, IoRestrictionNone,
"\\_SB.PCI0.GPIO", 0, ResourceConsumer, , ) { 12 }
PinConfig(Exclusive, 0x07, 0, "\\_SB.PCI0.GPIO", 0, ResourceConsumer, ) { 6 }
})
Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
Package () { "compatible", Package () { "gpio-sloppy-logic-analyzer" } },
Package () {
"probe-gpios", Package () {
^GSLA, 0, 0, 0,
^GSLA, 1, 0, 0,
},
},
Package () {
"probe-names", Package () {
"SCL",
"SDA",
},
},
}
})
}
> I am totally open to add ACPI but it
> should be tested, of course. Is there any on-going effort to add ACPI
> pin config?
Very slowly but yes, the pin configuration from ACPI to pin control is not
forgotten.
...
> > > + unsigned int trig_len;
> >
> > On 64-bit arch you may save 4 bytes by moving this to be together with u32
> > member above.
>
> I don't want to save bytes here. I sorted the struct for cachelines,
> important members first.
Add a comment then.
...
> > > +static struct dentry *gpio_la_poll_debug_dir;
> >
> > I have seen the idea of looking up the debugfs entry. That said, do we actually
> > need this global variable?
>
> I don't understand the first sentence. And we still need it to clean up?
If you know the name of the folder, you may look up it, no need to keep a
variable for that.
...
> > > + /* '10' is length of 'probe00=\n\0' */
> > > + add_len = strlen(gpio_names[i]) + 10;
> > > + meta = devm_krealloc(dev, meta, meta_len + add_len, GFP_KERNEL);
> >
> > First of all, this realloc() pattern *) is bad. While it's tricky and has side
> > effects (i.e. it has no leaks) better not to use it to avoid confusion.
> >
> > *) foo = realloc(foo, ...); is 101 mistake.
>
> Because generally you lose the old pointer on error. But we don't here
> because we are using managed devices.
>
> However, I see that all kernel users of devm_krealloc() are using a
> seperate variable and then update the old one. I can do this, too.
As I said, it is a nasty side effect that may provoke real bugs in the future
with simple realloc() cases.
...
> > > + [ ! -d $CPUSETDIR ] && mkdir $CPUSETDIR
> >
> > [ -d ... ] || ...
>
> Will think about it. I think the former is a tad more readable.
Shell is nice when the script is a) short, b) readable. Neither I see in the
former, sorry. Ah, and there is subtle difference between two. You may easily
learn it if you start using -efu flags in shebang.
--
With Best Regards,
Andy Shevchenko
