mbox series

[v7,0/3] Firmware Support for USB-HID Devices and CP2112

Message ID 20230223213147.268-1-kaehndan@gmail.com (mailing list archive)
Headers show
Series Firmware Support for USB-HID Devices and CP2112 | expand

Message

Daniel Kaehn Feb. 23, 2023, 9:31 p.m. UTC
This patchset allows USB-HID devices to have DeviceTree bindings through sharing
the USB fwnode with the HID driver, and adds such a binding and driver
implementation for the CP2112 USB to SMBus Bridge (which necessitated the
USB-HID change). This change allows a CP2112 permanently attached in hardware to
be described in DT and interoperate with other drivers.

Changes in v7:
- Use dev_fwnode when calling fwnod_handle_put in i2c_adapter in hid-cp2112.c
- Capitalize I2C and GPIO in commit message for patch 0003

Changes in v6:
- Fix fwnode_handle reference leaks in hid-cp21112.c
- Simplify hog node pattern in silabs,cp2112.yaml

Changes in v5:
 - Use fwnode API instead of of_node api in hid-core.c and hid-cp2112.c
 - Include sda-gpios and scl-gpios in silabs,cp2112.yaml
 - Additional fixups to silabs,cp2112.yaml to address comments
 - Submit threaded interrupt bugfix separately from this patchset, as requested

Changes in v4:
 - Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/i2c

Changes in v3:
 - Additional fixups to silabs,cp2112.yaml to address comments

Changes in v2:
 - Added more detail to silabs,cp2112.yaml dt-binding
 - Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/input
 - Added support for setting smbus clock-frequency from DT in hid-cp2112.c
 - Added freeing of of_nodes on error paths of _probe in hid-cp2112.c


Danny Kaehn (3):
  dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
  HID: usbhid: Share USB device firmware node with child HID device
  HID: cp2112: Fwnode Support

 .../bindings/i2c/silabs,cp2112.yaml           | 113 ++++++++++++++++++
 drivers/hid/hid-cp2112.c                      |  15 ++-
 drivers/hid/usbhid/hid-core.c                 |   2 +
 3 files changed, 128 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml

Comments

Andy Shevchenko Feb. 24, 2023, 4:43 p.m. UTC | #1
On Thu, Feb 23, 2023 at 03:31:44PM -0600, Danny Kaehn wrote:
> This patchset allows USB-HID devices to have DeviceTree bindings through sharing
> the USB fwnode with the HID driver, and adds such a binding and driver
> implementation for the CP2112 USB to SMBus Bridge (which necessitated the
> USB-HID change). This change allows a CP2112 permanently attached in hardware to
> be described in DT and interoperate with other drivers.

It's your responsibility to carry the tags you have got in the previous rounds
of the review. Please, be respectful to the reviewers who spent a lot of time
on yours, in particular, code. Otherwise why to bother with it (upstreaming)
at all?
Daniel Kaehn Feb. 24, 2023, 5:48 p.m. UTC | #2
On Fri, Feb 24, 2023 at 10:43 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Feb 23, 2023 at 03:31:44PM -0600, Danny Kaehn wrote:
> > This patchset allows USB-HID devices to have DeviceTree bindings through sharing
> > the USB fwnode with the HID driver, and adds such a binding and driver
> > implementation for the CP2112 USB to SMBus Bridge (which necessitated the
> > USB-HID change). This change allows a CP2112 permanently attached in hardware to
> > be described in DT and interoperate with other drivers.
>
> It's your responsibility to carry the tags you have got in the previous rounds
> of the review. Please, be respectful to the reviewers who spent a lot of time
> on yours, in particular, code. Otherwise why to bother with it (upstreaming)
> at all?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Hello Andy,

My sincerest apologies on this! I wasn't actually aware that this is
something I could do / am responsible for doing. No disrespect is
intended, though I see how this would be frustrating for reviewers (I
previously thought that maintainers used some sort of automated tool
to keep track of who approved/acked what in previous versions, but
didn't really know how that would work).

If I'm understanding correctly, this means that whenever someone
responds to my patch with a "Reviewed-by", etc.. I should be adding
that tag to the end of the commit message of that patch if a future
revision is needed? I assume this only applies on future revisions
where patches other than the one initially reviewed are changed, and
that any tags I take with should be dropped if that patch is changed?
Apologies about these questions - - I looked for guidance on this in
the various "submitting patches to the kernel" guides out there, and
wasn't able to find much.

Thanks,
Danny Kaehn
Andy Shevchenko Feb. 24, 2023, 8:56 p.m. UTC | #3
On Fri, Feb 24, 2023 at 11:48:02AM -0600, Daniel Kaehn wrote:
> On Fri, Feb 24, 2023 at 10:43 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Feb 23, 2023 at 03:31:44PM -0600, Danny Kaehn wrote:
> > > This patchset allows USB-HID devices to have DeviceTree bindings through sharing
> > > the USB fwnode with the HID driver, and adds such a binding and driver
> > > implementation for the CP2112 USB to SMBus Bridge (which necessitated the
> > > USB-HID change). This change allows a CP2112 permanently attached in hardware to
> > > be described in DT and interoperate with other drivers.
> >
> > It's your responsibility to carry the tags you have got in the previous rounds
> > of the review. Please, be respectful to the reviewers who spent a lot of time
> > on yours, in particular, code. Otherwise why to bother with it (upstreaming)
> > at all?

> Hello Andy,
> 
> My sincerest apologies on this! I wasn't actually aware that this is
> something I could do / am responsible for doing. No disrespect is
> intended, though I see how this would be frustrating for reviewers (I
> previously thought that maintainers used some sort of automated tool
> to keep track of who approved/acked what in previous versions, but
> didn't really know how that would work).

It's works only in the case if you have no more comments to address.
Indeed, `b4` tool may harvest tags from emails.

However, if you by your own initiative send a new version, to address
or change something, you need to take into account what was done in
the previous rounds of review. If you consider that tag can't be applied —
too many changes that doesn't match the code to the previous version(s), —
you should express why in the cover letter.

> If I'm understanding correctly, this means that whenever someone
> responds to my patch with a "Reviewed-by", etc.. I should be adding
> that tag to the end of the commit message of that patch if a future
> revision is needed?

Correct and you may use `b4` tool yourself to simplify that. It does
not require anything special (permissions, status, access, etc).

> I assume this only applies on future revisions
> where patches other than the one initially reviewed are changed, and
> that any tags I take with should be dropped if that patch is changed?

Depends on how big they are. In most cases the changes are not so drastically
big, so tags survive them.

> Apologies about these questions - - I looked for guidance on this in
> the various "submitting patches to the kernel" guides out there, and
> wasn't able to find much.

Understood. Now we will wait for v8 where you fix the mistakes.

Thank you for your patience and contribution!