| Message ID | e46ac7c1-1bb0-2caf-58e6-2fcaa89d30ae@gmail.com |
|---|---|
| Headers | show |
| Series | i2c: i801: Series with improvements | expand |
On Fri, Aug 06, 2021 at 11:12:18PM +0200, Heiner Kallweit wrote: > Setting the autosuspend delay to a negative value disables runtime pm in > a little bit smarter way, because we need no cleanup when removing the > driver. Note that this is safe when reloading the driver, because the > call to pm_runtime_set_autosuspend_delay() in probe() will reverse the > effect. See update_autosuspend() for details. > > Reviewed-by: Jean Delvare <jdelvare@suse.de> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied to for-next, thanks!
On Fri, Aug 06, 2021 at 11:14:08PM +0200, Heiner Kallweit wrote: > If a user is interested in such details he can enable smbus tracing. > > Reviewed-by: Jean Delvare <jdelvare@suse.de> > Tested-by: Jean Delvare <jdelvare@suse.de> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied to for-next, thanks! Waiting for further reviews for the following patches now.
On Fri, Aug 06, 2021 at 11:12:18PM +0200, Heiner Kallweit wrote: > Setting the autosuspend delay to a negative value disables runtime pm in > a little bit smarter way, because we need no cleanup when removing the > driver. Note that this is safe when reloading the driver, because the > call to pm_runtime_set_autosuspend_delay() in probe() will reverse the > effect. See update_autosuspend() for details. ... > * BIOS is accessing the host controller so prevent it from > * suspending automatically from now on. > */ > - pm_runtime_get_sync(&pdev->dev); > + pm_runtime_set_autosuspend_delay(&pdev->dev, -1); I dunno if it's being discussed, but with this you effectively allow user to override the setting. It may screw things up AFAIU the comment above. -- With Best Regards, Andy Shevchenko
On Fri, Aug 06, 2021 at 11:15:51PM +0200, Heiner Kallweit wrote: > do_pci_enable_device() takes care that PCI_COMMAND_INTX_DISABLE > is cleared if a legacy interrupt is used. Makes sense, but needs to be tested. Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/i2c/busses/i2c-i801.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c > index f56060fcf..7fa06b85f 100644 > --- a/drivers/i2c/busses/i2c-i801.c > +++ b/drivers/i2c/busses/i2c-i801.c > @@ -1827,19 +1827,12 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id) > priv->features &= ~FEATURE_IRQ; > > if (priv->features & FEATURE_IRQ) { > - u16 pcictl, pcists; > + u16 pcists; > > /* Complain if an interrupt is already pending */ > pci_read_config_word(priv->pci_dev, PCI_STATUS, &pcists); > if (pcists & PCI_STATUS_INTERRUPT) > dev_warn(&dev->dev, "An interrupt is pending!\n"); > - > - /* Check if interrupts have been disabled */ > - pci_read_config_word(priv->pci_dev, PCI_COMMAND, &pcictl); > - if (pcictl & PCI_COMMAND_INTX_DISABLE) { > - dev_info(&dev->dev, "Interrupts are disabled\n"); > - priv->features &= ~FEATURE_IRQ; > - } > } > > if (priv->features & FEATURE_IRQ) { > -- > 2.32.0 > > -- With Best Regards, Andy Shevchenko
On Fri, Aug 06, 2021 at 11:18:40PM +0200, Heiner Kallweit wrote: > The platform data structures are used in the respective i801_add_tco > functions only. Therefore we can make the definitions local to these > functions. Side note: usually we refer to the functions like "i801_add_tco()" (w/o quotes). -- With Best Regards, Andy Shevchenko
On 11.08.2021 17:41, Andy Shevchenko wrote: > On Fri, Aug 06, 2021 at 11:12:18PM +0200, Heiner Kallweit wrote: >> Setting the autosuspend delay to a negative value disables runtime pm in >> a little bit smarter way, because we need no cleanup when removing the >> driver. Note that this is safe when reloading the driver, because the >> call to pm_runtime_set_autosuspend_delay() in probe() will reverse the >> effect. See update_autosuspend() for details. > > ... > >> * BIOS is accessing the host controller so prevent it from >> * suspending automatically from now on. >> */ >> - pm_runtime_get_sync(&pdev->dev); >> + pm_runtime_set_autosuspend_delay(&pdev->dev, -1); > > I dunno if it's being discussed, but with this you effectively allow user to > override the setting. It may screw things up AFAIU the comment above. > No, this hasn't been discussed. At least not now. Thanks for the hint. This attribute is writable for the root user, so we could argue that the root user has several options to break the system anyway.
On Wed, Aug 11, 2021 at 10:03:12PM +0200, Heiner Kallweit wrote: > On 11.08.2021 17:41, Andy Shevchenko wrote: > > On Fri, Aug 06, 2021 at 11:12:18PM +0200, Heiner Kallweit wrote: > >> Setting the autosuspend delay to a negative value disables runtime pm in > >> a little bit smarter way, because we need no cleanup when removing the > >> driver. Note that this is safe when reloading the driver, because the > >> call to pm_runtime_set_autosuspend_delay() in probe() will reverse the > >> effect. See update_autosuspend() for details. > > > > ... > > > >> * BIOS is accessing the host controller so prevent it from > >> * suspending automatically from now on. > >> */ > >> - pm_runtime_get_sync(&pdev->dev); > >> + pm_runtime_set_autosuspend_delay(&pdev->dev, -1); > > > > I dunno if it's being discussed, but with this you effectively allow user to > > override the setting. It may screw things up AFAIU the comment above. > > > No, this hasn't been discussed. At least not now. Thanks for the hint. > This attribute is writable for the root user, so we could argue that > the root user has several options to break the system anyway. But it will mean the side effect on this driver and typical (root-run) system application (systemd like?) should care now the knowledge about this side-effect. I do not think it is desired behaviour. But I'm not a maintainer and I commented here just to make everybody understand the consequences of the change. -- With Best Regards, Andy Shevchenko
> > > I dunno if it's being discussed, but with this you effectively allow user to > > > override the setting. It may screw things up AFAIU the comment above. > > > > > No, this hasn't been discussed. At least not now. Thanks for the hint. > > This attribute is writable for the root user, so we could argue that > > the root user has several options to break the system anyway. > > But it will mean the side effect on this driver and typical (root-run) system > application (systemd like?) should care now the knowledge about this > side-effect. I do not think it is desired behaviour. But I'm not a maintainer > and I commented here just to make everybody understand the consequences of the > change. Jean, are you still fine with this patch then?
Hi Wolfram, On Tue, 17 Aug 2021 22:15:58 +0200, Wolfram Sang wrote: > > > > I dunno if it's being discussed, but with this you effectively allow user to > > > > override the setting. It may screw things up AFAIU the comment above. > > > > > > No, this hasn't been discussed. At least not now. Thanks for the hint. > > > This attribute is writable for the root user, so we could argue that > > > the root user has several options to break the system anyway. This is something we hear frequently when people don't want to address problems in their code, but that's not enough to convince me ;-) > > But it will mean the side effect on this driver and typical (root-run) system > > application (systemd like?) should care now the knowledge about this > > side-effect. I do not think it is desired behaviour. But I'm not a maintainer > > and I commented here just to make everybody understand the consequences of the > > change. Is systemd going to actually make any change to that attribute? I'm no systemd expert, but I can't see any option in the configuration files that would be related to autosuspend. > Jean, are you still fine with this patch then? My original position was that there are a few other drivers already doing "this". It's not like we are doing something completely new and using an API in a way it had never been used before, so it can't be that bad. On the other hand, after taking a closer look, I'm not fully certain that "this" is exactly the same in all these drivers. For example, in blk-pm.c, pm_runtime_set_autosuspend_delay() is being called with value -1 initially, but with the idea that someone else (device driver, user) may set a positive value later. It's not a permanent disable. The 8250_omap driver, however, seems to match the i2c-i801 driver here (I say "seems" because honestly I'm not sure I fully understand the comments there, but my understanding is that at least in some situations, enabling autosuspend later would cause problems). That being said, it starts looking like a problem for the PM subsystem maintainers. Basically Heiner is trying to move away from an API which requires cleaning up on driver removal. This is definitely the direction we are collectively taking for years now (the whole devm_* family of functions is about exactly that). So it's considered a good thing. If pm_runtime_set_autosuspend_delay() is not suitable for the task then maybe we need a better API. I will admit I'm at a loss when it comes to the many pm_runtime_* calls, I'm not going to claim I fully understand what each of them is doing exactly. But don't we want to simply call pm_runtime_dont_use_autosuspend() here? If not and there's no suitable API for the task at the moment, then better do not apply this patch, and instead ask the PM subsystem maintainers if they would be willing to implement what we need. -- Jean Delvare SUSE L3 Support
On Thu, Aug 26, 2021 at 04:00:21PM +0200, Jean Delvare wrote: > On Tue, 17 Aug 2021 22:15:58 +0200, Wolfram Sang wrote: Jean, thanks for your response. My 2 cents below. > > > > > I dunno if it's being discussed, but with this you effectively allow user to > > > > > override the setting. It may screw things up AFAIU the comment above. > > > > > > > > No, this hasn't been discussed. At least not now. Thanks for the hint. > > > > This attribute is writable for the root user, so we could argue that > > > > the root user has several options to break the system anyway. > > This is something we hear frequently when people don't want to address > problems in their code, but that's not enough to convince me ;-) > > > > But it will mean the side effect on this driver and typical (root-run) system > > > application (systemd like?) should care now the knowledge about this > > > side-effect. I do not think it is desired behaviour. But I'm not a maintainer > > > and I commented here just to make everybody understand the consequences of the > > > change. > > Is systemd going to actually make any change to that attribute? I'm no > systemd expert, but I can't see any option in the configuration files > that would be related to autosuspend. > > > Jean, are you still fine with this patch then? > > My original position was that there are a few other drivers already > doing "this". It's not like we are doing something completely new and > using an API in a way it had never been used before, so it can't be > that bad. > > On the other hand, after taking a closer look, I'm not fully certain > that "this" is exactly the same in all these drivers. For example, in > blk-pm.c, pm_runtime_set_autosuspend_delay() is being called with value > -1 initially, but with the idea that someone else (device driver, user) > may set a positive value later. It's not a permanent disable. Correct, it's expected that either system wide tool or user with enough privileges may enable it. > The 8250_omap driver, Oh, no, please, do not use that driver as an example for runtime PM. It's (somehow) broken there! I Cc'ed Tony in case you have more Q:s about it. It's on a healing curve, though. The idea behind I believe is the same, i.e. to allow user to turn it on and off. > however, seems to match the i2c-i801 driver here (I > say "seems" because honestly I'm not sure I fully understand the > comments there, but my understanding is that at least in some > situations, enabling autosuspend later would cause problems). > > That being said, it starts looking like a problem for the PM subsystem > maintainers. Basically Heiner is trying to move away from an API which > requires cleaning up on driver removal. This is definitely the > direction we are collectively taking for years now (the whole devm_* > family of functions is about exactly that). So it's considered a good > thing. > > If pm_runtime_set_autosuspend_delay() is not suitable for the task then > maybe we need a better API. I will admit I'm at a loss when it comes to > the many pm_runtime_* calls, I'm not going to claim I fully understand > what each of them is doing exactly. But don't we want to simply call > pm_runtime_dont_use_autosuspend() here? > > If not and there's no suitable API for the task at the moment, then > better do not apply this patch, and instead ask the PM subsystem > maintainers if they would be willing to implement what we need. -- With Best Regards, Andy Shevchenko
On Fri, 06 Aug 2021 23:15:51 +0200, Heiner Kallweit wrote: > do_pci_enable_device() takes care that PCI_COMMAND_INTX_DISABLE > is cleared if a legacy interrupt is used. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/i2c/busses/i2c-i801.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c > index f56060fcf..7fa06b85f 100644 > --- a/drivers/i2c/busses/i2c-i801.c > +++ b/drivers/i2c/busses/i2c-i801.c > @@ -1827,19 +1827,12 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id) > priv->features &= ~FEATURE_IRQ; > > if (priv->features & FEATURE_IRQ) { > - u16 pcictl, pcists; > + u16 pcists; > > /* Complain if an interrupt is already pending */ > pci_read_config_word(priv->pci_dev, PCI_STATUS, &pcists); > if (pcists & PCI_STATUS_INTERRUPT) > dev_warn(&dev->dev, "An interrupt is pending!\n"); > - > - /* Check if interrupts have been disabled */ > - pci_read_config_word(priv->pci_dev, PCI_COMMAND, &pcictl); > - if (pcictl & PCI_COMMAND_INTX_DISABLE) { > - dev_info(&dev->dev, "Interrupts are disabled\n"); > - priv->features &= ~FEATURE_IRQ; > - } > } > > if (priv->features & FEATURE_IRQ) { Reviewed-by: Jean Delvare <jdelvare@suse.de> As said before, I'm not sure, but let's apply that and see if anybody complains. -- Jean Delvare SUSE L3 Support
On 26.08.2021 16:00, Jean Delvare wrote: > Hi Wolfram, > > On Tue, 17 Aug 2021 22:15:58 +0200, Wolfram Sang wrote: >>>>> I dunno if it's being discussed, but with this you effectively allow user to >>>>> override the setting. It may screw things up AFAIU the comment above. >>>> >>>> No, this hasn't been discussed. At least not now. Thanks for the hint. >>>> This attribute is writable for the root user, so we could argue that >>>> the root user has several options to break the system anyway. > > This is something we hear frequently when people don't want to address > problems in their code, but that's not enough to convince me ;-) > >>> But it will mean the side effect on this driver and typical (root-run) system >>> application (systemd like?) should care now the knowledge about this >>> side-effect. I do not think it is desired behaviour. But I'm not a maintainer >>> and I commented here just to make everybody understand the consequences of the >>> change. > > Is systemd going to actually make any change to that attribute? I'm no > systemd expert, but I can't see any option in the configuration files > that would be related to autosuspend. > >> Jean, are you still fine with this patch then? > > My original position was that there are a few other drivers already > doing "this". It's not like we are doing something completely new and > using an API in a way it had never been used before, so it can't be > that bad. > > On the other hand, after taking a closer look, I'm not fully certain > that "this" is exactly the same in all these drivers. For example, in > blk-pm.c, pm_runtime_set_autosuspend_delay() is being called with value > -1 initially, but with the idea that someone else (device driver, user) > may set a positive value later. It's not a permanent disable. The > 8250_omap driver, however, seems to match the i2c-i801 driver here (I > say "seems" because honestly I'm not sure I fully understand the > comments there, but my understanding is that at least in some > situations, enabling autosuspend later would cause problems). > > That being said, it starts looking like a problem for the PM subsystem > maintainers. Basically Heiner is trying to move away from an API which > requires cleaning up on driver removal. This is definitely the > direction we are collectively taking for years now (the whole devm_* > family of functions is about exactly that). So it's considered a good > thing. > > If pm_runtime_set_autosuspend_delay() is not suitable for the task then > maybe we need a better API. I will admit I'm at a loss when it comes to > the many pm_runtime_* calls, I'm not going to claim I fully understand > what each of them is doing exactly. But don't we want to simply call > pm_runtime_dont_use_autosuspend() here? > > If not and there's no suitable API for the task at the moment, then > better do not apply this patch, and instead ask the PM subsystem > maintainers if they would be willing to implement what we need. > To follow-up on this: This patch has been applied already. Therefore, if decision is to not go with it, it would need to be reverted.
On Tue, 31 Aug 2021 08:05:41 +0200, Heiner Kallweit wrote: > On 26.08.2021 16:00, Jean Delvare wrote: > > If pm_runtime_set_autosuspend_delay() is not suitable for the task then > > maybe we need a better API. I will admit I'm at a loss when it comes to > > the many pm_runtime_* calls, I'm not going to claim I fully understand > > what each of them is doing exactly. But don't we want to simply call > > pm_runtime_dont_use_autosuspend() here? > > > > If not and there's no suitable API for the task at the moment, then > > better do not apply this patch, and instead ask the PM subsystem > > maintainers if they would be willing to implement what we need. > > To follow-up on this: This patch has been applied already. Therefore, > if decision is to not go with it, it would need to be reverted. Technically it's not in Linus' tree yet ;-) I'm still interested to know if pm_runtime_dont_use_autosuspend() is the right call to use in this situation. -- Jean Delvare SUSE L3 Support
On 31.08.2021 13:26, Jean Delvare wrote: > On Tue, 31 Aug 2021 08:05:41 +0200, Heiner Kallweit wrote: >> On 26.08.2021 16:00, Jean Delvare wrote: >>> If pm_runtime_set_autosuspend_delay() is not suitable for the task then >>> maybe we need a better API. I will admit I'm at a loss when it comes to >>> the many pm_runtime_* calls, I'm not going to claim I fully understand >>> what each of them is doing exactly. But don't we want to simply call >>> pm_runtime_dont_use_autosuspend() here? >>> >>> If not and there's no suitable API for the task at the moment, then >>> better do not apply this patch, and instead ask the PM subsystem >>> maintainers if they would be willing to implement what we need. >> >> To follow-up on this: This patch has been applied already. Therefore, >> if decision is to not go with it, it would need to be reverted. > > Technically it's not in Linus' tree yet ;-) > > I'm still interested to know if pm_runtime_dont_use_autosuspend() is > the right call to use in this situation. > I don't think so. It disable auto-suspending, but leaves "normal" runtime-suspending active. Calling pm_runtime_disable() may be an alternative. Or we use the following to re-establish the old behavior with a little less overhead. Getting the mutex isn't needed here because the PCI core increments the rpm usage_count before calling the remove() hook. diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 1f929e6c3..b5723d946 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -1623,7 +1623,7 @@ i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits, * BIOS is accessing the host controller so prevent it from * suspending automatically from now on. */ - pm_runtime_set_autosuspend_delay(&pdev->dev, -1); + pm_runtime_get_sync(&pdev->dev); } if ((function & ACPI_IO_MASK) == ACPI_READ) @@ -1891,7 +1891,9 @@ static void i801_remove(struct pci_dev *dev) struct i801_priv *priv = pci_get_drvdata(dev); pm_runtime_forbid(&dev->dev); - pm_runtime_get_noresume(&dev->dev); + /* if acpi_reserved is set then usage_count is incremented already */ + if (!priv->acpi_reserved) + pm_runtime_get_noresume(&dev->dev); i801_disable_host_notify(priv); i801_del_mux(priv); -- 2.33.0
On 31.08.2021 22:43, Heiner Kallweit wrote: > On 31.08.2021 13:26, Jean Delvare wrote: >> On Tue, 31 Aug 2021 08:05:41 +0200, Heiner Kallweit wrote: >>> On 26.08.2021 16:00, Jean Delvare wrote: >>>> If pm_runtime_set_autosuspend_delay() is not suitable for the task then >>>> maybe we need a better API. I will admit I'm at a loss when it comes to >>>> the many pm_runtime_* calls, I'm not going to claim I fully understand >>>> what each of them is doing exactly. But don't we want to simply call >>>> pm_runtime_dont_use_autosuspend() here? >>>> >>>> If not and there's no suitable API for the task at the moment, then >>>> better do not apply this patch, and instead ask the PM subsystem >>>> maintainers if they would be willing to implement what we need. >>> >>> To follow-up on this: This patch has been applied already. Therefore, >>> if decision is to not go with it, it would need to be reverted. >> >> Technically it's not in Linus' tree yet ;-) >> >> I'm still interested to know if pm_runtime_dont_use_autosuspend() is >> the right call to use in this situation. >> > I don't think so. It disable auto-suspending, but leaves "normal" > runtime-suspending active. Calling pm_runtime_disable() may be an > alternative. > Or we use the following to re-establish the old behavior with a little > less overhead. Getting the mutex isn't needed here because the PCI > core increments the rpm usage_count before calling the remove() hook. > Just figured out that what I proposed wasn't fully correct. We should only access priv->acpi_reserved once we're sure the ACPI io handler can't run in parallel. Small disclaimer: I'm not fully sure how acpi_remove_address_space_handler() behaves if it's called whilst the handler is running. IOW: Whether we can be sure that after the call to acpi_remove_address_space_handler() the handler isn't running. On a sidenote: At least the call to pm_runtime_forbid() isn't needed because runtime pm is disabled anyway and the calls to pm_runtime_forbid/allow don't have to be balanced. diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c index 1f929e6c3..6394c8340 100644 --- a/drivers/i2c/busses/i2c-i801.c +++ b/drivers/i2c/busses/i2c-i801.c @@ -1623,7 +1623,7 @@ i801_acpi_io_handler(u32 function, acpi_physical_address address, u32 bits, * BIOS is accessing the host controller so prevent it from * suspending automatically from now on. */ - pm_runtime_set_autosuspend_delay(&pdev->dev, -1); + pm_runtime_get_sync(&pdev->dev); } if ((function & ACPI_IO_MASK) == ACPI_READ) @@ -1890,9 +1890,6 @@ static void i801_remove(struct pci_dev *dev) { struct i801_priv *priv = pci_get_drvdata(dev); - pm_runtime_forbid(&dev->dev); - pm_runtime_get_noresume(&dev->dev); - i801_disable_host_notify(priv); i801_del_mux(priv); i2c_del_adapter(&priv->adapter); @@ -1901,6 +1898,10 @@ static void i801_remove(struct pci_dev *dev) platform_device_unregister(priv->tco_pdev); + pm_runtime_forbid(&dev->dev); + /* if acpi_reserved is set then usage_count is incremented already */ + if (!priv->acpi_reserved) + pm_runtime_get_noresume(&dev->dev); /* * do not call pci_disable_device(dev) since it can cause hard hangs on * some systems during power-off (eg. Fujitsu-Siemens Lifebook E8010) -- 2.33.0
On Fri, Aug 06, 2021 at 11:15:51PM +0200, Heiner Kallweit wrote: > do_pci_enable_device() takes care that PCI_COMMAND_INTX_DISABLE > is cleared if a legacy interrupt is used. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied to for-next, thanks!
On Fri, Aug 06, 2021 at 11:17:10PM +0200, Heiner Kallweit wrote: > The return value of i801_add_mux() isn't used, so let's change it to void. > In addition remove the not needed cast to struct gpiod_lookup. > GPIO_LOOKUP() uses GPIO_LOOKUP_IDX() that includes this cast. > > Reviewed-by: Jean Delvare <jdelvare@suse.de> > Tested-by: Jean Delvare <jdelvare@suse.de> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied to for-next, thanks!
On Fri, Aug 06, 2021 at 11:18:40PM +0200, Heiner Kallweit wrote: > The platform data structures are used in the respective i801_add_tco > functions only. Therefore we can make the definitions local to these > functions. > > Reviewed-by: Jean Delvare <jdelvare@suse.de> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Hmm, this doesn't apply on i2c/for-mergewindow. Did I miss a patch?
On Sat, Oct 02, 2021 at 09:44:55AM +0200, Wolfram Sang wrote: > On Fri, Aug 06, 2021 at 11:18:40PM +0200, Heiner Kallweit wrote: > > The platform data structures are used in the respective i801_add_tco > > functions only. Therefore we can make the definitions local to these > > functions. > > > > Reviewed-by: Jean Delvare <jdelvare@suse.de> > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > Hmm, this doesn't apply on i2c/for-mergewindow. Did I miss a patch? Since a lot of other i801 patches have been merged meanwhile, I'd need this patch rebased if you still want it to be applied. Thanks for all your i801 work!
On 29.11.2021 09:58, Wolfram Sang wrote: > On Sat, Oct 02, 2021 at 09:44:55AM +0200, Wolfram Sang wrote: >> On Fri, Aug 06, 2021 at 11:18:40PM +0200, Heiner Kallweit wrote: >>> The platform data structures are used in the respective i801_add_tco >>> functions only. Therefore we can make the definitions local to these >>> functions. >>> >>> Reviewed-by: Jean Delvare <jdelvare@suse.de> >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> >> Hmm, this doesn't apply on i2c/for-mergewindow. Did I miss a patch? > > Since a lot of other i801 patches have been merged meanwhile, I'd need > this patch rebased if you still want it to be applied. Thanks for all > your i801 work! > I just sent a rebased version.