On Sat, 30 Dec 2023, Lukas Wunner wrote:
> On Fri, Sep 29, 2023 at 02:57:22PM +0300, Ilpo Järvinen wrote:
> > @@ -253,8 +255,16 @@ static int pcie_bandwidth_notification_probe(struct pcie_device *srv)
> > pcie_enable_link_bandwidth_notification(port);
> > pci_info(port, "enabled with IRQ %d\n", srv->irq);
> >
> > + data->cdev = pcie_cooling_device_register(port, srv);
> > + if (IS_ERR(data->cdev)) {
> > + ret = PTR_ERR(data->cdev);
> > + goto disable_notifications;
> > + }
> > return 0;
>
> Now wait a minute, if you can't register the cooling device,
> you still want to provide accurate link speeds to the user
> in sysfs, right? At least that's what you promise in Kconfig.
When thermal side is not even configured, it returns NULL which is not
ERR.
I guess I can change the behavior for the real ERR cases (I was bit on
borderline what to do with those failures).
> > --- /dev/null
> > +++ b/drivers/thermal/pcie_cooling.c
> > @@ -0,0 +1,107 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PCIe cooling device
> > + *
> > + * Copyright (C) 2023 Intel Corporation.
>
> Another trailing period I'd remove.
>
> I take it this patch (only) allows manual bandwidth throttling
> through sysfs, right? And emergency throttling is introduced
> separately on top of this?
Only sysfs throttling is introduced by this series, there's no emergency
throttling in the series. Also, many things have been simplified in this
series because more complex things would be only justified with
the emergency throttling and would just raise questions 'why is this and
that being done' (e.g., the critical section was enlarged as per your
request where if there would be emergency throttlink doesn't make sense to
wait until the end of the link training before "emergency throttling" can
attempt to lower the link speed).
--
i.
