On Wed, Nov 16, 2022 at 09:11:58AM +0200, Ivaylo Dimitrov wrote: > > > On 14.11.22 г. 18:46 ч., Ivaylo Dimitrov wrote: > > Hi, > > > > On 14.11.22 г. 18:14 ч., Greg KH wrote: > > > On Mon, Nov 14, 2022 at 02:56:02PM +0200, Ivaylo Dimitrov wrote: > > > > usb_phy::notifier is already used by various PHY drivers (including > > > > phy_generic) to report VBUS status changes and its usage conflicts with > > > > charger current limit changes reporting. > > > > > > How exactly does it conflict? > > > > > > > see below > > > > > > Fix that by introducing a second notifier that is dedicated to > > > > usb charger > > > > notifications. Add usb_charger_XXX_notifier functions. Fix > > > > charger drivers > > > > that currently (ab)use usb_XXX_notifier() to use the new API. > > > > > > Why not just set the notifier type to be a new one instead of adding a > > > whole new notifier list? Or use a real callback? notifier lists are > > > really horrid and should be avoided whenever possible. > > > > > > > Not sure what you mean by "notifier type', but if that is that val > > parameter of atomic_notifier_call_chain(), the way it is used by usb > > charger FW: > > > > https://elixir.bootlin.com/linux/latest/source/drivers/usb/phy/phy.c#L132 > > > > is not compatible with: > > > > https://elixir.bootlin.com/linux/latest/source/drivers/usb/phy/phy-generic.c#L185 > > > > > > for example, IIUC. > > > > The former wants to send max current as val, while latter sends event > > type as val. Sure, I may create some kind of hack, like using the MSB to > > denote charger events, but that doesn't feel right. > > > > Or, shall I do something else and fix the usage all over the place? > > Please elaborate. > > > > Digging further into that, it seems phy-ab8500-usb.c is also using > usb_phy::notifier in non-standard way, it sends events from > ux500_musb_vbus_id_status instead of usb_phy_events. I don't know the > history behind, but right now we have at least 3 incompatible usages of > usb_phy::notifier: > > 1. Most of the phy and charger drivers use usb_phy_events as notifier type > > 2. phy-ab8500-usb.c uses ux500_musb_vbus_id_status as notifier type, I am > not the only one to hit that it seems https://elixir.bootlin.com/linux/v6.1-rc5/source/drivers/power/supply/ab8500_charger.c#L3191 > > 3. USB charger framework uses max charging current as notifier type. > > Moreover, a charger driver in a system that has gadget drivers support and > phy that has extcon charger cable detection support and registers to phy > notifier, will inevitably receive (1) and (3) types of notifications, > without any way to distinguish I was able to find. Can't they properly detect this based on the type of the notification sent to them? Why not just set that correctly? > I don't really see how those can be merged to use one notifier only, without > fixing most of USB phy and gadget drivers and half of charger drivers. Not > that I like adding the second notifier, I just don;t see other way. Fixing them all so that we don't have this mess and require yet-another-notifier would be very good. I know it's not your mess, but I think it's the best long-term solution to it, don't you? thanks, greg k-h
