All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Daniel Kaehn <kaehndan@gmail.com>,
	Hans de Goede <hdegoede@redhat.com>,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	jikos@kernel.org, bartosz.golaszewski@linaro.org,
	dmitry.torokhov@gmail.com, devicetree@vger.kernel.org,
	linux-input@vger.kernel.org, ethan.twardy@plexus.com
Subject: Re: [PATCH v8 3/3] HID: cp2112: Fwnode Support
Date: Mon, 6 Mar 2023 15:07:19 +0200	[thread overview]
Message-ID: <ZAXlh9ZVjGJh0l7n@smile.fi.intel.com> (raw)
In-Reply-To: <b8423b0b-4f63-d598-6c8b-7c7e73549032@redhat.com>

On Mon, Mar 06, 2023 at 01:36:51PM +0100, Benjamin Tissoires wrote:
> On Mon, Mar 6, 2023 at 11:49 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Mar 02, 2023 at 06:06:06PM +0100, Benjamin Tissoires wrote:
> > > On Mar 01 2023, Andy Shevchenko wrote:
> > > > On Tue, Feb 28, 2023 at 01:05:54PM -0600, Daniel Kaehn wrote:

...

[1]: https://stackoverflow.com/a/60855157/2511795

> > > Thanks Andy for your help here, and thanks for that link.
> > >
> > > I am trying to test Danny's patch as I want to use it for my HID CI,
> > > being an owner of a CP2112 device myself.
> > >
> > > The current setup is using out of the tree patches [2] which are
> > > implementing a platform i2c-hid support and some manual addition of a
> > > I2C-HID device on top of it. This works fine but gets busted every now
> > > and then when the tree sees a change that conflicts with these patches.
> > >
> > > So with Danny's series, I thought I could have an SSDT override to
> > > declare that very same device instead of patching my kernel before
> > > testing it.
> > >
> > > Of course, it gets tricky because I need to run that under qemu.
> > >
> > > I am currently stuck at the "sharing the firmware_node from usb with
> > > HID" step and I'd like to know if you could help me.
> > >
> > > On my laptop, if I plug the CP2112 (without using a USB hub), I can get:
> > >
> > > $> ls -l /sys/bus/hid/devices/0003:10C4:EA90.*
> > >   lrwxrwxrwx. 1 root root 0 Mar  2 17:02 /sys/bus/hid/devices/0003:10C4:EA90.0079 -> ../../../devices/pci0000:00/0000:00:14.0/usb2/2-9/2-9:1.0/0003:10C4:EA90.0079
> > > $> ls -l /sys/bus/usb/devices/2-9*/firmware_node
> > >   lrwxrwxrwx. 1 root root 0 Mar  2 17:03 /sys/bus/usb/devices/2-9:1.0/firmware_node -> ../../../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:15/device:16/device:25
> > >   lrwxrwxrwx. 1 root root 0 Mar  2 17:02 /sys/bus/usb/devices/2-9/firmware_node -> ../../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:15/device:16/device:25
> > >
> > > So AFAIU the USB device is properly assigned a firmware node. My dsdt
> > > also shows the "Device (RHUB)" and I guess everything is fine.
> > 
> > Yes, so far so good.
> > 
> > > However, playing with qemu is not so easy.
> > >
> > > I am running qemu with the following arguments (well, almost because I
> > > have a wrapper script on top of it and I also run the compiled kernel
> > > from the current tree):
> > >
> > > #> qemu-system-x86_64 -bios /usr/share/edk2/ovmf/OVMF_CODE.fd \
> > >                       -netdev user,id=hostnet0 \
> > >                       -device virtio-net-pci,netdev=hostnet0 \
> > >                       -m 4G \
> > >                       -enable-kvm \
> > >                       -cpu host \
> > >                       -device qemu-xhci -usb \
> > >                       -device 'usb-host,vendorid=0x10c4,productid=0xea90' \
> > >                       -cdrom ~/Downloads/Fedora-Workstation-Live-x86_64-37-1.7.iso
> > 
> > Side question, where can I get those blobs from (EDKII and Fedora Live CD)?
> > I'm using Debian unstable.
> 
> You can install the ovmf package in debian[3], which should have a
> similar file.
> For the Fedora livecd -> https://getfedora.org/fr/workstation/download/
> but any other distribution with a recent enough kernel should show the
> same.

Thank you!

> > > And this is what I get:
> > >
> > > #> ls -l /sys/bus/hid/devices/0003:10C4:EA90.*
> > >   lrwxrwxrwx 1 root root 0 Mar  2 16:10 /sys/bus/hid/devices/0003:10C4:EA90.0001 -> ../../../devices/pci0000:00/0000:00:06.0/usb2/2-1/2-1:1.0/0003:10C4:EA90.0001
> > >
> > > #> ls -l /sys/bus/usb/devices/2-1*/firmware_node
> > >   ls: cannot access '/sys/bus/usb/devices/2-1*/firmware_node': No such file or directory
> > >
> > > Looking at the DSDT, I do not see any reference to the USB hub, so I
> > > wonder if the firmware_node needs to be populated first in the DSDT.
> > 
> > So, where QEMU takes DSDT (ACPI tables in general) from? Can you patch that?
> > I believe that's the problem in qemu.
> 
> That's a good question and it's one I am not sure I have the answer to.
> I would have assumed that the DSDT was in the OVMF firmware, but given
> that we can arbitrarily add command line arguments, I believe it
> probably just provides a baseline and then we are screwed. The OVMF bios
> is compiled only once, so I doubt there is any mechanism to
> enable/disable a component in the DSDT, or make it dynamically
> generated.

We have two ways of filling missing parts:
1) update the original source of DSDT (firmware or bootloader,
   whichever provides that);
2) adding an overlay.

The 2) works _if and only if_ there is *no* existing object in the tables.
In such cases, you can simply provide a *full* hierarchy. See an example of
PCI devices in the kernel documentation on how to do that. I believe something
similar can be done for USB.

> > > Also note that if I plug the CP2112 over a docking station, I lose the
> > > firmware_node sysfs entries on the host too.
> > 
> > This seems like a lack of firmware node propagating in the USB hub code in
> > the Linux kernel.
> 
> That would make a lot of sense.
> 
> FWIW, in the VM I see a firmware node on the pci controller itself:
> #> ls -l /sys/devices/pci0000\:00/0000\:00\:06.0/firmware_node
>   lrwxrwxrwx 1 root root 0 Mar  6 12:24 /sys/devices/pci0000:00/0000:00:06.0/firmware_node -> ../../LNXSYSTM:00/LNXSYBUS:00/PNP0A03:00/device:07
> 
> And one the host, through a USB hub:
> 
> #> ls -l /sys/bus/hid/devices/0003:10C4:EA90.*
>   lrwxrwxrwx. 1 root root 0 Mar  6 13:26 /sys/bus/hid/devices/0003:10C4:EA90.007C -> ../../../devices/pci0000:00/0000:00:14.0/usb2/2-8/2-8.2/2-8.2.4/2-8.2.4:1.0/0003:10C4:EA90.007C
> #> ls -l /sys/bus/usb/devices/2-8*/firmware_node
>   lrwxrwxrwx. 1 root root 0 Mar  2 16:53 /sys/bus/usb/devices/2-8:1.0/firmware_node -> ../../../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:15/device:16/device:1e
>   lrwxrwxrwx. 1 root root 0 Mar  2 16:53 /sys/bus/usb/devices/2-8/firmware_node -> ../../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:15/device:16/device:1e
> 
> Note that the firmware node propagation stopped at 2-8, and 2.8.2 is not
> having a firmware node.

It would be nice if you can run `grep -H 15 /sys/bus/acpi/devices/*/status`,
filter out unneeded ones, and for the rest also print their paths:
`cat filtered_list_of_acpi_devs | while read p; do grep -H . $p/path; done`

With this we will see what devices are actually present and up and running
in the system and what their paths in the ACPI namespace.

> > > Do you think it would be achievable to emulate that over qemu and use a
> > > mainline kernel without patches?
> > 
> > As long as qemu provides correct DSDT it should work I assume.
> 
> Just to be sure I understand, for this to work, we need the DSDT to
> export a "Device(RHUB)"?

Not sure I understand the term "export" here. We need a description
of the (to describe) missing parts.

> Or if we fix the USB fw_node propagation, could we just overwrite
> "\_SB_.PCI0.S30_"?  "\_SB_.PCI0.S30_" is the name the ACPI is giving to
> the USB port in my VM case AFAIU.

I have no idea what is the S30 node.

[2] https://gitlab.freedesktop.org/bentiss/gitlab-kernel-ci/-/tree/master/VM
[3] https://packages.debian.org/buster/all/ovmf/filelist

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2023-03-06 13:07 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-27 14:07 [PATCH v8 0/3] Firmware Support for USB-HID Devices and CP2112 Danny Kaehn
2023-02-27 14:07 ` [PATCH v8 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge Danny Kaehn
2023-02-27 14:07 ` [PATCH v8 2/3] HID: usbhid: Share USB device firmware node with child HID device Danny Kaehn
2023-02-27 14:07 ` [PATCH v8 3/3] HID: cp2112: Fwnode Support Danny Kaehn
2023-02-27 23:07   ` Andy Shevchenko
2023-02-28 19:05     ` Daniel Kaehn
2023-03-01 14:59       ` Andy Shevchenko
2023-03-01 15:04         ` Andy Shevchenko
2023-03-02 17:06         ` Benjamin Tissoires
2023-03-06 10:49           ` Andy Shevchenko
2023-03-06 12:36             ` Benjamin Tissoires
2023-03-06 13:07               ` Andy Shevchenko [this message]
2023-03-06 14:48                 ` Benjamin Tissoires
2023-03-06 17:01                   ` Andy Shevchenko
2023-03-06 19:40                     ` Daniel Kaehn
2023-03-06 20:36                       ` Andy Shevchenko
2023-03-07 13:17                         ` Benjamin Tissoires
2023-03-07 13:53                           ` Daniel Kaehn
2023-03-07 14:48                             ` Benjamin Tissoires
2023-03-07 18:18                               ` Andy Shevchenko
2023-03-08 15:26                                 ` Benjamin Tissoires
2023-03-08 15:37                                   ` Daniel Kaehn
2023-03-08 15:55                                     ` Benjamin Tissoires
2023-03-08 16:30                                       ` Andy Shevchenko
2023-03-08 16:36                                         ` Andy Shevchenko
2023-03-08 18:32                                           ` Daniel Kaehn
2023-03-09 11:43                                             ` Andy Shevchenko
2023-03-09 11:56                                               ` Andy Shevchenko
2023-03-09  9:38                                         ` Benjamin Tissoires
2023-03-09 13:50                                           ` Andy Shevchenko
2023-03-08 16:20                                   ` Andy Shevchenko
2023-03-07 18:15                             ` Andy Shevchenko
2023-03-07 18:14                           ` Andy Shevchenko
2023-03-07 19:57                             ` Daniel Kaehn
2023-03-08 12:41                               ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZAXlh9ZVjGJh0l7n@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=bartosz.golaszewski@linaro.org \
    --cc=benjamin.tissoires@redhat.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=ethan.twardy@plexus.com \
    --cc=hdegoede@redhat.com \
    --cc=jikos@kernel.org \
    --cc=kaehndan@gmail.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-input@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.