* Re: [PATCH 3/5] libceph: crush_location infrastructure
From: Jeff Layton @ 2020-05-29 19:10 UTC (permalink / raw)
To: Ilya Dryomov; +Cc: Ceph Development
In-Reply-To: <CAOi1vP-UPktbA7gcx0tQvN9wi1L2DB1zHyb212x5kbErkchR=Q@mail.gmail.com>
On Fri, 2020-05-29 at 20:38 +0200, Ilya Dryomov wrote:
> On Fri, May 29, 2020 at 7:27 PM Jeff Layton <jlayton@kernel.org> wrote:
> > On Fri, 2020-05-29 at 17:19 +0200, Ilya Dryomov wrote:
> > > Allow expressing client's location in terms of CRUSH hierarchy as
> > > a set of (bucket type name, bucket name) pairs. The userspace syntax
> > > "crush_location = key1=value1 key2=value2" is incompatible with mount
> > > options and needed adaptation:
> > >
> > > - ':' separator
> > > - one key:value pair per crush_location option
> > > - crush_location options are combined together
> > >
> > > So for:
> > >
> > > crush_location = host=foo rack=bar
> > >
> > > one would write:
> > >
> > > crush_location=host:foo,crush_location=rack:bar
> > >
> > > As in userspace, "multipath" locations are supported, so indicating
> > > locality for parallel hierarchies is possible:
> > >
> > > crush_location=rack:foo1,crush_location=rack:foo2,crush_location=datacenter:bar
> > >
> >
> > Blech, that syntax is hideous. It's also problematic in that the options
> > are additive -- you can't override an option that was given earlier
> > (e.g. in fstab), or in a shell script.
> >
> > Is it not possible to do something with a single crush_location= option?
> > Maybe:
> >
> > crush_location=rack:foo1/rack:foo2/datacenter:bar
> >
> > It's still ugly with the embedded '=' signs, but it would at least make
> > it so that the options aren't additive.
>
> I suppose we could do something like that at the cost of more
> parsing boilerplate, but I'm not sure additive options are that
> hideous. I don't think additive options are unprecedented and
> more importantly I think many simple boolean and integer options
> are not properly overridable even in major filesystems.
>
That is the long-standing convention though. There are reasons to
deviate from it, but I don't see it here. Plus, I think the syntax I
proposed above is more readable (and compact) as well.
It would mean a bit more parsing code though, granted.
> What embedded '=' signs are you referring to? I see ':' and '/'
> in your suggested syntax.
>
Sorry, yeah... I had originally done one that had '=' chars in it, but
converted it to the above. Please disregard that paragraph.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 05/11] thermal: remove get_mode() operation of drivers
From: Guenter Roeck @ 2020-05-29 19:10 UTC (permalink / raw)
To: Andrzej Pietrasiewicz
Cc: linux-pm, linux-acpi, netdev, linux-wireless, platform-driver-x86,
linux-arm-kernel, linux-renesas-soc, linux-rockchip,
Emmanuel Grumbach, Heiko Stuebner, Rafael J . Wysocki,
Vishal Kulkarni, Luca Coelho, Miquel Raynal, kernel,
Fabio Estevam, Amit Kucheria, Chunyan Zhang, Daniel Lezcano,
Allison Randal, NXP Linux Team, Darren Hart, Zhang Rui,
Gayatri Kammela, Len Brown, Johannes Berg, Intel Linux Wireless,
Sascha Hauer, Ido Schimmel, Baolin Wang, Jiri Pirko, Orson Zhai,
Thomas Gleixner, Kalle Valo, Support Opensource, Enrico Weigelt,
Peter Kaestle, Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Niklas Söderlund, Shawn Guo,
David S . Miller, Andy Shevchenko
On Thu, May 28, 2020 at 09:20:45PM +0200, Andrzej Pietrasiewicz wrote:
> get_mode() is now redundant, as the state is stored in struct
> thermal_zone_device.
>
> Consequently the "mode" attribute in sysfs can always be visible, because
> it is always possible to get the mode from struct tzd.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
Based on separate feedback/dscussion:
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Guenter
> ---
> drivers/acpi/thermal.c | 9 ------
> .../ethernet/mellanox/mlxsw/core_thermal.c | 19 ------------
> drivers/platform/x86/acerhdf.c | 12 --------
> drivers/thermal/da9062-thermal.c | 8 -----
> drivers/thermal/imx_thermal.c | 9 ------
> .../intel/int340x_thermal/int3400_thermal.c | 9 ------
> .../thermal/intel/intel_quark_dts_thermal.c | 8 -----
> drivers/thermal/thermal_core.c | 7 +----
> drivers/thermal/thermal_of.c | 9 ------
> drivers/thermal/thermal_sysfs.c | 30 ++-----------------
> include/linux/thermal.h | 2 --
> 11 files changed, 3 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 4ba273f49d87..592be97c4456 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -525,14 +525,6 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
> return 0;
> }
>
> -static int thermal_get_mode(struct thermal_zone_device *thermal,
> - enum thermal_device_mode *mode)
> -{
> - *mode = thermal->mode;
> -
> - return 0;
> -}
> -
> static int thermal_set_mode(struct thermal_zone_device *thermal,
> enum thermal_device_mode mode)
> {
> @@ -847,7 +839,6 @@ static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> .bind = acpi_thermal_bind_cooling_device,
> .unbind = acpi_thermal_unbind_cooling_device,
> .get_temp = thermal_get_temp,
> - .get_mode = thermal_get_mode,
> .set_mode = thermal_set_mode,
> .get_trip_type = thermal_get_trip_type,
> .get_trip_temp = thermal_get_trip_temp,
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index aa082e8a0b13..6e26678ac312 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -275,14 +275,6 @@ static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
> return 0;
> }
>
> -static int mlxsw_thermal_get_mode(struct thermal_zone_device *tzdev,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tzdev->mode;
> -
> - return 0;
> -}
> -
> static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
> enum thermal_device_mode mode)
> {
> @@ -403,7 +395,6 @@ static int mlxsw_thermal_trend_get(struct thermal_zone_device *tzdev,
> static struct thermal_zone_device_ops mlxsw_thermal_ops = {
> .bind = mlxsw_thermal_bind,
> .unbind = mlxsw_thermal_unbind,
> - .get_mode = mlxsw_thermal_get_mode,
> .set_mode = mlxsw_thermal_set_mode,
> .get_temp = mlxsw_thermal_get_temp,
> .get_trip_type = mlxsw_thermal_get_trip_type,
> @@ -462,14 +453,6 @@ static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
> return err;
> }
>
> -static int mlxsw_thermal_module_mode_get(struct thermal_zone_device *tzdev,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tzdev->mode;
> -
> - return 0;
> -}
> -
> static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
> enum thermal_device_mode mode)
> {
> @@ -591,7 +574,6 @@ mlxsw_thermal_module_trip_hyst_set(struct thermal_zone_device *tzdev, int trip,
> static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
> .bind = mlxsw_thermal_module_bind,
> .unbind = mlxsw_thermal_module_unbind,
> - .get_mode = mlxsw_thermal_module_mode_get,
> .set_mode = mlxsw_thermal_module_mode_set,
> .get_temp = mlxsw_thermal_module_temp_get,
> .get_trip_type = mlxsw_thermal_module_trip_type_get,
> @@ -630,7 +612,6 @@ static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device *tzdev,
> static struct thermal_zone_device_ops mlxsw_thermal_gearbox_ops = {
> .bind = mlxsw_thermal_module_bind,
> .unbind = mlxsw_thermal_module_unbind,
> - .get_mode = mlxsw_thermal_module_mode_get,
> .set_mode = mlxsw_thermal_module_mode_set,
> .get_temp = mlxsw_thermal_gearbox_temp_get,
> .get_trip_type = mlxsw_thermal_module_trip_type_get,
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 97b288485837..32c5fe16b7f7 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -413,17 +413,6 @@ static inline void acerhdf_enable_kernelmode(void)
> pr_notice("kernel mode fan control ON\n");
> }
>
> -static int acerhdf_get_mode(struct thermal_zone_device *thermal,
> - enum thermal_device_mode *mode)
> -{
> - if (verbose)
> - pr_notice("kernel mode fan control %d\n", kernelmode);
> -
> - *mode = thermal->mode;
> -
> - return 0;
> -}
> -
> /*
> * set operation mode;
> * enabled: the thermal layer of the kernel takes care about
> @@ -490,7 +479,6 @@ static struct thermal_zone_device_ops acerhdf_dev_ops = {
> .bind = acerhdf_bind,
> .unbind = acerhdf_unbind,
> .get_temp = acerhdf_get_ec_temp,
> - .get_mode = acerhdf_get_mode,
> .set_mode = acerhdf_set_mode,
> .get_trip_type = acerhdf_get_trip_type,
> .get_trip_hyst = acerhdf_get_trip_hyst,
> diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
> index a14c7981c7c7..a7ac8afb063e 100644
> --- a/drivers/thermal/da9062-thermal.c
> +++ b/drivers/thermal/da9062-thermal.c
> @@ -120,13 +120,6 @@ static irqreturn_t da9062_thermal_irq_handler(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> -static int da9062_thermal_get_mode(struct thermal_zone_device *z,
> - enum thermal_device_mode *mode)
> -{
> - *mode = z->mode;
> - return 0;
> -}
> -
> static int da9062_thermal_get_trip_type(struct thermal_zone_device *z,
> int trip,
> enum thermal_trip_type *type)
> @@ -179,7 +172,6 @@ static int da9062_thermal_get_temp(struct thermal_zone_device *z,
>
> static struct thermal_zone_device_ops da9062_thermal_ops = {
> .get_temp = da9062_thermal_get_temp,
> - .get_mode = da9062_thermal_get_mode,
> .get_trip_type = da9062_thermal_get_trip_type,
> .get_trip_temp = da9062_thermal_get_trip_temp,
> };
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 9a1114d721b6..2c7ee5da608a 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -330,14 +330,6 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> return 0;
> }
>
> -static int imx_get_mode(struct thermal_zone_device *tz,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tz->mode;
> -
> - return 0;
> -}
> -
> static int imx_set_mode(struct thermal_zone_device *tz,
> enum thermal_device_mode mode)
> {
> @@ -464,7 +456,6 @@ static struct thermal_zone_device_ops imx_tz_ops = {
> .bind = imx_bind,
> .unbind = imx_unbind,
> .get_temp = imx_get_temp,
> - .get_mode = imx_get_mode,
> .set_mode = imx_set_mode,
> .get_trip_type = imx_get_trip_type,
> .get_trip_temp = imx_get_trip_temp,
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index f65b2fc09198..9a622aaf29dd 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -377,14 +377,6 @@ static int int3400_thermal_get_temp(struct thermal_zone_device *thermal,
> return 0;
> }
>
> -static int int3400_thermal_get_mode(struct thermal_zone_device *thermal,
> - enum thermal_device_mode *mode)
> -{
> - *mode = thermal->mode;
> -
> - return 0;
> -}
> -
> static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
> enum thermal_device_mode mode)
> {
> @@ -412,7 +404,6 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
>
> static struct thermal_zone_device_ops int3400_thermal_ops = {
> .get_temp = int3400_thermal_get_temp,
> - .get_mode = int3400_thermal_get_mode,
> .set_mode = int3400_thermal_set_mode,
> };
>
> diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
> index d77cb3df5ade..c4879b4bfbf1 100644
> --- a/drivers/thermal/intel/intel_quark_dts_thermal.c
> +++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
> @@ -308,13 +308,6 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd,
> return 0;
> }
>
> -static int sys_get_mode(struct thermal_zone_device *tzd,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tzd->mode;
> - return 0;
> -}
> -
> static int sys_set_mode(struct thermal_zone_device *tzd,
> enum thermal_device_mode mode)
> {
> @@ -336,7 +329,6 @@ static struct thermal_zone_device_ops tzone_ops = {
> .get_trip_type = sys_get_trip_type,
> .set_trip_temp = sys_set_trip_temp,
> .get_crit_temp = sys_get_crit_temp,
> - .get_mode = sys_get_mode,
> .set_mode = sys_set_mode,
> };
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index b71196eaf90e..14d3b1b94c4f 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1456,7 +1456,6 @@ static int thermal_pm_notify(struct notifier_block *nb,
> unsigned long mode, void *_unused)
> {
> struct thermal_zone_device *tz;
> - enum thermal_device_mode tz_mode;
>
> switch (mode) {
> case PM_HIBERNATION_PREPARE:
> @@ -1469,11 +1468,7 @@ static int thermal_pm_notify(struct notifier_block *nb,
> case PM_POST_SUSPEND:
> atomic_set(&in_suspend, 0);
> list_for_each_entry(tz, &thermal_tz_list, node) {
> - tz_mode = THERMAL_DEVICE_ENABLED;
> - if (tz->ops->get_mode)
> - tz->ops->get_mode(tz, &tz_mode);
> -
> - if (tz_mode == THERMAL_DEVICE_DISABLED)
> + if (tz->mode == THERMAL_DEVICE_DISABLED)
> continue;
>
> thermal_zone_device_init(tz);
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index c495b1e48ef2..ba65d48a48cb 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -267,14 +267,6 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
> return 0;
> }
>
> -static int of_thermal_get_mode(struct thermal_zone_device *tz,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tz->mode;
> -
> - return 0;
> -}
> -
> static int of_thermal_set_mode(struct thermal_zone_device *tz,
> enum thermal_device_mode mode)
> {
> @@ -389,7 +381,6 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
> }
>
> static struct thermal_zone_device_ops of_thermal_ops = {
> - .get_mode = of_thermal_get_mode,
> .set_mode = of_thermal_set_mode,
>
> .get_trip_type = of_thermal_get_trip_type,
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index aa99edb4dff7..096370977068 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -49,18 +49,9 @@ static ssize_t
> mode_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> struct thermal_zone_device *tz = to_thermal_zone(dev);
> - enum thermal_device_mode mode;
> - int result;
> -
> - if (!tz->ops->get_mode)
> - return -EPERM;
>
> - result = tz->ops->get_mode(tz, &mode);
> - if (result)
> - return result;
> -
> - return sprintf(buf, "%s\n", mode == THERMAL_DEVICE_ENABLED ? "enabled"
> - : "disabled");
> + return sprintf(buf, "%s\n", tz->mode == THERMAL_DEVICE_ENABLED ?
> + "enabled" : "disabled");
> }
>
> static ssize_t
> @@ -428,30 +419,13 @@ static struct attribute_group thermal_zone_attribute_group = {
> .attrs = thermal_zone_dev_attrs,
> };
>
> -/* We expose mode only if .get_mode is present */
> static struct attribute *thermal_zone_mode_attrs[] = {
> &dev_attr_mode.attr,
> NULL,
> };
>
> -static umode_t thermal_zone_mode_is_visible(struct kobject *kobj,
> - struct attribute *attr,
> - int attrno)
> -{
> - struct device *dev = container_of(kobj, struct device, kobj);
> - struct thermal_zone_device *tz;
> -
> - tz = container_of(dev, struct thermal_zone_device, device);
> -
> - if (tz->ops->get_mode)
> - return attr->mode;
> -
> - return 0;
> -}
> -
> static struct attribute_group thermal_zone_mode_attribute_group = {
> .attrs = thermal_zone_mode_attrs,
> - .is_visible = thermal_zone_mode_is_visible,
> };
>
> /* We expose passive only if passive trips are present */
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 5f91d7f04512..a808f6fa2777 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -76,8 +76,6 @@ struct thermal_zone_device_ops {
> struct thermal_cooling_device *);
> int (*get_temp) (struct thermal_zone_device *, int *);
> int (*set_trips) (struct thermal_zone_device *, int, int);
> - int (*get_mode) (struct thermal_zone_device *,
> - enum thermal_device_mode *);
> int (*set_mode) (struct thermal_zone_device *,
> enum thermal_device_mode);
> int (*get_trip_type) (struct thermal_zone_device *, int,
^ permalink raw reply
* Re: [PATCH v4 05/11] thermal: remove get_mode() operation of drivers
From: Guenter Roeck @ 2020-05-29 19:10 UTC (permalink / raw)
To: Andrzej Pietrasiewicz
Cc: linux-pm, linux-acpi, netdev, linux-wireless, platform-driver-x86,
linux-arm-kernel, linux-renesas-soc, linux-rockchip,
Emmanuel Grumbach, Heiko Stuebner, Rafael J . Wysocki,
Vishal Kulkarni, Luca Coelho, Miquel Raynal, kernel,
Fabio Estevam, Amit Kucheria, Chunyan Zhang, Daniel Lezcano,
Allison Randal, NXP Linux Team
On Thu, May 28, 2020 at 09:20:45PM +0200, Andrzej Pietrasiewicz wrote:
> get_mode() is now redundant, as the state is stored in struct
> thermal_zone_device.
>
> Consequently the "mode" attribute in sysfs can always be visible, because
> it is always possible to get the mode from struct tzd.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
Based on separate feedback/dscussion:
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Guenter
> ---
> drivers/acpi/thermal.c | 9 ------
> .../ethernet/mellanox/mlxsw/core_thermal.c | 19 ------------
> drivers/platform/x86/acerhdf.c | 12 --------
> drivers/thermal/da9062-thermal.c | 8 -----
> drivers/thermal/imx_thermal.c | 9 ------
> .../intel/int340x_thermal/int3400_thermal.c | 9 ------
> .../thermal/intel/intel_quark_dts_thermal.c | 8 -----
> drivers/thermal/thermal_core.c | 7 +----
> drivers/thermal/thermal_of.c | 9 ------
> drivers/thermal/thermal_sysfs.c | 30 ++-----------------
> include/linux/thermal.h | 2 --
> 11 files changed, 3 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 4ba273f49d87..592be97c4456 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -525,14 +525,6 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
> return 0;
> }
>
> -static int thermal_get_mode(struct thermal_zone_device *thermal,
> - enum thermal_device_mode *mode)
> -{
> - *mode = thermal->mode;
> -
> - return 0;
> -}
> -
> static int thermal_set_mode(struct thermal_zone_device *thermal,
> enum thermal_device_mode mode)
> {
> @@ -847,7 +839,6 @@ static struct thermal_zone_device_ops acpi_thermal_zone_ops = {
> .bind = acpi_thermal_bind_cooling_device,
> .unbind = acpi_thermal_unbind_cooling_device,
> .get_temp = thermal_get_temp,
> - .get_mode = thermal_get_mode,
> .set_mode = thermal_set_mode,
> .get_trip_type = thermal_get_trip_type,
> .get_trip_temp = thermal_get_trip_temp,
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index aa082e8a0b13..6e26678ac312 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -275,14 +275,6 @@ static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
> return 0;
> }
>
> -static int mlxsw_thermal_get_mode(struct thermal_zone_device *tzdev,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tzdev->mode;
> -
> - return 0;
> -}
> -
> static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
> enum thermal_device_mode mode)
> {
> @@ -403,7 +395,6 @@ static int mlxsw_thermal_trend_get(struct thermal_zone_device *tzdev,
> static struct thermal_zone_device_ops mlxsw_thermal_ops = {
> .bind = mlxsw_thermal_bind,
> .unbind = mlxsw_thermal_unbind,
> - .get_mode = mlxsw_thermal_get_mode,
> .set_mode = mlxsw_thermal_set_mode,
> .get_temp = mlxsw_thermal_get_temp,
> .get_trip_type = mlxsw_thermal_get_trip_type,
> @@ -462,14 +453,6 @@ static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
> return err;
> }
>
> -static int mlxsw_thermal_module_mode_get(struct thermal_zone_device *tzdev,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tzdev->mode;
> -
> - return 0;
> -}
> -
> static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
> enum thermal_device_mode mode)
> {
> @@ -591,7 +574,6 @@ mlxsw_thermal_module_trip_hyst_set(struct thermal_zone_device *tzdev, int trip,
> static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
> .bind = mlxsw_thermal_module_bind,
> .unbind = mlxsw_thermal_module_unbind,
> - .get_mode = mlxsw_thermal_module_mode_get,
> .set_mode = mlxsw_thermal_module_mode_set,
> .get_temp = mlxsw_thermal_module_temp_get,
> .get_trip_type = mlxsw_thermal_module_trip_type_get,
> @@ -630,7 +612,6 @@ static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device *tzdev,
> static struct thermal_zone_device_ops mlxsw_thermal_gearbox_ops = {
> .bind = mlxsw_thermal_module_bind,
> .unbind = mlxsw_thermal_module_unbind,
> - .get_mode = mlxsw_thermal_module_mode_get,
> .set_mode = mlxsw_thermal_module_mode_set,
> .get_temp = mlxsw_thermal_gearbox_temp_get,
> .get_trip_type = mlxsw_thermal_module_trip_type_get,
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 97b288485837..32c5fe16b7f7 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -413,17 +413,6 @@ static inline void acerhdf_enable_kernelmode(void)
> pr_notice("kernel mode fan control ON\n");
> }
>
> -static int acerhdf_get_mode(struct thermal_zone_device *thermal,
> - enum thermal_device_mode *mode)
> -{
> - if (verbose)
> - pr_notice("kernel mode fan control %d\n", kernelmode);
> -
> - *mode = thermal->mode;
> -
> - return 0;
> -}
> -
> /*
> * set operation mode;
> * enabled: the thermal layer of the kernel takes care about
> @@ -490,7 +479,6 @@ static struct thermal_zone_device_ops acerhdf_dev_ops = {
> .bind = acerhdf_bind,
> .unbind = acerhdf_unbind,
> .get_temp = acerhdf_get_ec_temp,
> - .get_mode = acerhdf_get_mode,
> .set_mode = acerhdf_set_mode,
> .get_trip_type = acerhdf_get_trip_type,
> .get_trip_hyst = acerhdf_get_trip_hyst,
> diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
> index a14c7981c7c7..a7ac8afb063e 100644
> --- a/drivers/thermal/da9062-thermal.c
> +++ b/drivers/thermal/da9062-thermal.c
> @@ -120,13 +120,6 @@ static irqreturn_t da9062_thermal_irq_handler(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> -static int da9062_thermal_get_mode(struct thermal_zone_device *z,
> - enum thermal_device_mode *mode)
> -{
> - *mode = z->mode;
> - return 0;
> -}
> -
> static int da9062_thermal_get_trip_type(struct thermal_zone_device *z,
> int trip,
> enum thermal_trip_type *type)
> @@ -179,7 +172,6 @@ static int da9062_thermal_get_temp(struct thermal_zone_device *z,
>
> static struct thermal_zone_device_ops da9062_thermal_ops = {
> .get_temp = da9062_thermal_get_temp,
> - .get_mode = da9062_thermal_get_mode,
> .get_trip_type = da9062_thermal_get_trip_type,
> .get_trip_temp = da9062_thermal_get_trip_temp,
> };
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 9a1114d721b6..2c7ee5da608a 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -330,14 +330,6 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> return 0;
> }
>
> -static int imx_get_mode(struct thermal_zone_device *tz,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tz->mode;
> -
> - return 0;
> -}
> -
> static int imx_set_mode(struct thermal_zone_device *tz,
> enum thermal_device_mode mode)
> {
> @@ -464,7 +456,6 @@ static struct thermal_zone_device_ops imx_tz_ops = {
> .bind = imx_bind,
> .unbind = imx_unbind,
> .get_temp = imx_get_temp,
> - .get_mode = imx_get_mode,
> .set_mode = imx_set_mode,
> .get_trip_type = imx_get_trip_type,
> .get_trip_temp = imx_get_trip_temp,
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index f65b2fc09198..9a622aaf29dd 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -377,14 +377,6 @@ static int int3400_thermal_get_temp(struct thermal_zone_device *thermal,
> return 0;
> }
>
> -static int int3400_thermal_get_mode(struct thermal_zone_device *thermal,
> - enum thermal_device_mode *mode)
> -{
> - *mode = thermal->mode;
> -
> - return 0;
> -}
> -
> static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
> enum thermal_device_mode mode)
> {
> @@ -412,7 +404,6 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
>
> static struct thermal_zone_device_ops int3400_thermal_ops = {
> .get_temp = int3400_thermal_get_temp,
> - .get_mode = int3400_thermal_get_mode,
> .set_mode = int3400_thermal_set_mode,
> };
>
> diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
> index d77cb3df5ade..c4879b4bfbf1 100644
> --- a/drivers/thermal/intel/intel_quark_dts_thermal.c
> +++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
> @@ -308,13 +308,6 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd,
> return 0;
> }
>
> -static int sys_get_mode(struct thermal_zone_device *tzd,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tzd->mode;
> - return 0;
> -}
> -
> static int sys_set_mode(struct thermal_zone_device *tzd,
> enum thermal_device_mode mode)
> {
> @@ -336,7 +329,6 @@ static struct thermal_zone_device_ops tzone_ops = {
> .get_trip_type = sys_get_trip_type,
> .set_trip_temp = sys_set_trip_temp,
> .get_crit_temp = sys_get_crit_temp,
> - .get_mode = sys_get_mode,
> .set_mode = sys_set_mode,
> };
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index b71196eaf90e..14d3b1b94c4f 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1456,7 +1456,6 @@ static int thermal_pm_notify(struct notifier_block *nb,
> unsigned long mode, void *_unused)
> {
> struct thermal_zone_device *tz;
> - enum thermal_device_mode tz_mode;
>
> switch (mode) {
> case PM_HIBERNATION_PREPARE:
> @@ -1469,11 +1468,7 @@ static int thermal_pm_notify(struct notifier_block *nb,
> case PM_POST_SUSPEND:
> atomic_set(&in_suspend, 0);
> list_for_each_entry(tz, &thermal_tz_list, node) {
> - tz_mode = THERMAL_DEVICE_ENABLED;
> - if (tz->ops->get_mode)
> - tz->ops->get_mode(tz, &tz_mode);
> -
> - if (tz_mode == THERMAL_DEVICE_DISABLED)
> + if (tz->mode == THERMAL_DEVICE_DISABLED)
> continue;
>
> thermal_zone_device_init(tz);
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index c495b1e48ef2..ba65d48a48cb 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -267,14 +267,6 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
> return 0;
> }
>
> -static int of_thermal_get_mode(struct thermal_zone_device *tz,
> - enum thermal_device_mode *mode)
> -{
> - *mode = tz->mode;
> -
> - return 0;
> -}
> -
> static int of_thermal_set_mode(struct thermal_zone_device *tz,
> enum thermal_device_mode mode)
> {
> @@ -389,7 +381,6 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz,
> }
>
> static struct thermal_zone_device_ops of_thermal_ops = {
> - .get_mode = of_thermal_get_mode,
> .set_mode = of_thermal_set_mode,
>
> .get_trip_type = of_thermal_get_trip_type,
> diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
> index aa99edb4dff7..096370977068 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -49,18 +49,9 @@ static ssize_t
> mode_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> struct thermal_zone_device *tz = to_thermal_zone(dev);
> - enum thermal_device_mode mode;
> - int result;
> -
> - if (!tz->ops->get_mode)
> - return -EPERM;
>
> - result = tz->ops->get_mode(tz, &mode);
> - if (result)
> - return result;
> -
> - return sprintf(buf, "%s\n", mode == THERMAL_DEVICE_ENABLED ? "enabled"
> - : "disabled");
> + return sprintf(buf, "%s\n", tz->mode == THERMAL_DEVICE_ENABLED ?
> + "enabled" : "disabled");
> }
>
> static ssize_t
> @@ -428,30 +419,13 @@ static struct attribute_group thermal_zone_attribute_group = {
> .attrs = thermal_zone_dev_attrs,
> };
>
> -/* We expose mode only if .get_mode is present */
> static struct attribute *thermal_zone_mode_attrs[] = {
> &dev_attr_mode.attr,
> NULL,
> };
>
> -static umode_t thermal_zone_mode_is_visible(struct kobject *kobj,
> - struct attribute *attr,
> - int attrno)
> -{
> - struct device *dev = container_of(kobj, struct device, kobj);
> - struct thermal_zone_device *tz;
> -
> - tz = container_of(dev, struct thermal_zone_device, device);
> -
> - if (tz->ops->get_mode)
> - return attr->mode;
> -
> - return 0;
> -}
> -
> static struct attribute_group thermal_zone_mode_attribute_group = {
> .attrs = thermal_zone_mode_attrs,
> - .is_visible = thermal_zone_mode_is_visible,
> };
>
> /* We expose passive only if passive trips are present */
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 5f91d7f04512..a808f6fa2777 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -76,8 +76,6 @@ struct thermal_zone_device_ops {
> struct thermal_cooling_device *);
> int (*get_temp) (struct thermal_zone_device *, int *);
> int (*set_trips) (struct thermal_zone_device *, int, int);
> - int (*get_mode) (struct thermal_zone_device *,
> - enum thermal_device_mode *);
> int (*set_mode) (struct thermal_zone_device *,
> enum thermal_device_mode);
> int (*get_trip_type) (struct thermal_zone_device *, int,
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: thermal: rcar-thermal: Add device tree support for r8a7742
From: Rob Herring @ 2020-05-29 19:09 UTC (permalink / raw)
To: Lad Prabhakar
Cc: linux-kernel, Magnus Damm, Geert Uytterhoeven, Rob Herring,
Prabhakar, linux-renesas-soc, devicetree
In-Reply-To: <1590614320-30160-2-git-send-email-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Wed, 27 May 2020 22:18:37 +0100, Lad Prabhakar wrote:
> Add thermal sensor support for r8a7742 SoC. The Renesas RZ/G1H
> (r8a7742) thermal sensor module is identical to the R-Car Gen2 family.
>
> No driver change is needed due to the fallback compatible value
> "renesas,rcar-gen2-thermal".
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> ---
> Documentation/devicetree/bindings/thermal/rcar-thermal.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH v4 04/11] thermal: Store device mode in struct thermal_zone_device
From: Guenter Roeck @ 2020-05-29 19:09 UTC (permalink / raw)
To: Andrzej Pietrasiewicz
Cc: linux-pm, linux-acpi, netdev, linux-wireless, platform-driver-x86,
linux-arm-kernel, linux-renesas-soc, linux-rockchip,
Emmanuel Grumbach, Heiko Stuebner, Rafael J . Wysocki,
Vishal Kulkarni, Luca Coelho, Miquel Raynal, kernel,
Fabio Estevam, Amit Kucheria, Chunyan Zhang, Daniel Lezcano,
Allison Randal, NXP Linux Team, Darren Hart, Zhang Rui,
Gayatri Kammela, Len Brown, Johannes Berg, Intel Linux Wireless,
Sascha Hauer, Ido Schimmel, Baolin Wang, Jiri Pirko, Orson Zhai,
Thomas Gleixner, Kalle Valo, Support Opensource, Enrico Weigelt,
Peter Kaestle, Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Niklas Söderlund, Shawn Guo,
David S . Miller, Andy Shevchenko
On Thu, May 28, 2020 at 09:20:44PM +0200, Andrzej Pietrasiewicz wrote:
> Prepare for eliminating get_mode().
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
Based on discussion:
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Guenter
> ---
> drivers/acpi/thermal.c | 18 ++++++----------
> .../ethernet/mellanox/mlxsw/core_thermal.c | 21 +++++++------------
> drivers/platform/x86/acerhdf.c | 15 ++++++-------
> drivers/thermal/da9062-thermal.c | 6 ++----
> drivers/thermal/imx_thermal.c | 17 +++++++--------
> .../intel/int340x_thermal/int3400_thermal.c | 12 +++--------
> .../thermal/intel/intel_quark_dts_thermal.c | 16 +++++++-------
> drivers/thermal/thermal_of.c | 10 +++------
> 8 files changed, 44 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index fb46070c66d8..4ba273f49d87 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -172,7 +172,6 @@ struct acpi_thermal {
> struct acpi_thermal_trips trips;
> struct acpi_handle_list devices;
> struct thermal_zone_device *thermal_zone;
> - enum thermal_device_mode mode;
> int kelvin_offset; /* in millidegrees */
> struct work_struct thermal_check_work;
> };
> @@ -500,7 +499,7 @@ static void acpi_thermal_check(void *data)
> {
> struct acpi_thermal *tz = data;
>
> - if (tz->mode != THERMAL_DEVICE_ENABLED)
> + if (tz->thermal_zone->mode != THERMAL_DEVICE_ENABLED)
> return;
>
> thermal_zone_device_update(tz->thermal_zone,
> @@ -529,12 +528,7 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
> static int thermal_get_mode(struct thermal_zone_device *thermal,
> enum thermal_device_mode *mode)
> {
> - struct acpi_thermal *tz = thermal->devdata;
> -
> - if (!tz)
> - return -EINVAL;
> -
> - *mode = tz->mode;
> + *mode = thermal->mode;
>
> return 0;
> }
> @@ -556,11 +550,11 @@ static int thermal_set_mode(struct thermal_zone_device *thermal,
> if (mode == THERMAL_DEVICE_DISABLED)
> pr_warn("thermal zone will be disabled\n");
>
> - if (mode != tz->mode) {
> - tz->mode = mode;
> + if (mode != tz->thermal_zone->mode) {
> + tz->thermal_zone->mode = mode;
> ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> "%s kernel ACPI thermal control\n",
> - tz->mode == THERMAL_DEVICE_ENABLED ?
> + tz->thermal_zone->mode == THERMAL_DEVICE_ENABLED ?
> "Enable" : "Disable"));
> acpi_thermal_check(tz);
> }
> @@ -912,7 +906,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
> goto remove_dev_link;
> }
>
> - tz->mode = THERMAL_DEVICE_ENABLED;
> + tz->thermal_zone->mode = THERMAL_DEVICE_ENABLED;
>
> dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
> tz->thermal_zone->id);
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index ce0a6837daa3..aa082e8a0b13 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -98,7 +98,6 @@ struct mlxsw_thermal_module {
> struct mlxsw_thermal *parent;
> struct thermal_zone_device *tzdev;
> struct mlxsw_thermal_trip trips[MLXSW_THERMAL_NUM_TRIPS];
> - enum thermal_device_mode mode;
> int module; /* Module or gearbox number */
> };
>
> @@ -110,7 +109,6 @@ struct mlxsw_thermal {
> struct thermal_cooling_device *cdevs[MLXSW_MFCR_PWMS_MAX];
> u8 cooling_levels[MLXSW_THERMAL_MAX_STATE + 1];
> struct mlxsw_thermal_trip trips[MLXSW_THERMAL_NUM_TRIPS];
> - enum thermal_device_mode mode;
> struct mlxsw_thermal_module *tz_module_arr;
> u8 tz_module_num;
> struct mlxsw_thermal_module *tz_gearbox_arr;
> @@ -280,9 +278,7 @@ static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
> static int mlxsw_thermal_get_mode(struct thermal_zone_device *tzdev,
> enum thermal_device_mode *mode)
> {
> - struct mlxsw_thermal *thermal = tzdev->devdata;
> -
> - *mode = thermal->mode;
> + *mode = tzdev->mode;
>
> return 0;
> }
> @@ -299,9 +295,9 @@ static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
> else
> tzdev->polling_delay = 0;
>
> + tzdev->mode = mode;
> mutex_unlock(&tzdev->lock);
>
> - thermal->mode = mode;
> thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
>
> return 0;
> @@ -469,9 +465,7 @@ static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
> static int mlxsw_thermal_module_mode_get(struct thermal_zone_device *tzdev,
> enum thermal_device_mode *mode)
> {
> - struct mlxsw_thermal_module *tz = tzdev->devdata;
> -
> - *mode = tz->mode;
> + *mode = tzdev->mode;
>
> return 0;
> }
> @@ -489,9 +483,10 @@ static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
> else
> tzdev->polling_delay = 0;
>
> + tzdev->mode = mode;
> +
> mutex_unlock(&tzdev->lock);
>
> - tz->mode = mode;
> thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
>
> return 0;
> @@ -765,7 +760,7 @@ mlxsw_thermal_module_tz_init(struct mlxsw_thermal_module *module_tz)
> return err;
> }
>
> - module_tz->mode = THERMAL_DEVICE_ENABLED;
> + module_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
> return 0;
> }
>
> @@ -881,7 +876,7 @@ mlxsw_thermal_gearbox_tz_init(struct mlxsw_thermal_module *gearbox_tz)
> if (IS_ERR(gearbox_tz->tzdev))
> return PTR_ERR(gearbox_tz->tzdev);
>
> - gearbox_tz->mode = THERMAL_DEVICE_ENABLED;
> + gearbox_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
> return 0;
> }
>
> @@ -1050,7 +1045,7 @@ int mlxsw_thermal_init(struct mlxsw_core *core,
> if (err)
> goto err_unreg_modules_tzdev;
>
> - thermal->mode = THERMAL_DEVICE_ENABLED;
> + thermal->tzdev->mode = THERMAL_DEVICE_ENABLED;
> *p_thermal = thermal;
> return 0;
>
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 830a8b060e74..97b288485837 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -68,7 +68,6 @@ static int kernelmode = 1;
> #else
> static int kernelmode;
> #endif
> -static enum thermal_device_mode thermal_mode;
>
> static unsigned int interval = 10;
> static unsigned int fanon = 60000;
> @@ -398,15 +397,16 @@ static inline void acerhdf_revert_to_bios_mode(void)
> {
> acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
> kernelmode = 0;
> - thermal_mode = THERMAL_DEVICE_DISABLED;
> - if (thz_dev)
> + if (thz_dev) {
> + thz_dev->mode = THERMAL_DEVICE_DISABLED;
> thz_dev->polling_delay = 0;
> + }
> pr_notice("kernel mode fan control OFF\n");
> }
> static inline void acerhdf_enable_kernelmode(void)
> {
> kernelmode = 1;
> - thermal_mode = THERMAL_DEVICE_ENABLED;
> + thz_dev->mode = THERMAL_DEVICE_ENABLED;
>
> thz_dev->polling_delay = interval*1000;
> thermal_zone_device_update(thz_dev, THERMAL_EVENT_UNSPECIFIED);
> @@ -419,7 +419,7 @@ static int acerhdf_get_mode(struct thermal_zone_device *thermal,
> if (verbose)
> pr_notice("kernel mode fan control %d\n", kernelmode);
>
> - *mode = thermal_mode;
> + *mode = thermal->mode;
>
> return 0;
> }
> @@ -741,8 +741,6 @@ static int __init acerhdf_register_thermal(void)
> if (IS_ERR(cl_dev))
> return -EINVAL;
>
> - thermal_mode = kernelmode ?
> - THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED;
> thz_dev = thermal_zone_device_register("acerhdf", 2, 0, NULL,
> &acerhdf_dev_ops,
> &acerhdf_zone_params, 0,
> @@ -750,6 +748,9 @@ static int __init acerhdf_register_thermal(void)
> if (IS_ERR(thz_dev))
> return -EINVAL;
>
> + thz_dev->mode = kernelmode ?
> + THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED;
> +
> if (strcmp(thz_dev->governor->name,
> acerhdf_zone_params.governor_name)) {
> pr_err("Didn't get thermal governor %s, perhaps not compiled into thermal subsystem.\n",
> diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
> index c32709badeda..a14c7981c7c7 100644
> --- a/drivers/thermal/da9062-thermal.c
> +++ b/drivers/thermal/da9062-thermal.c
> @@ -49,7 +49,6 @@ struct da9062_thermal {
> struct da9062 *hw;
> struct delayed_work work;
> struct thermal_zone_device *zone;
> - enum thermal_device_mode mode;
> struct mutex lock; /* protection for da9062_thermal temperature */
> int temperature;
> int irq;
> @@ -124,8 +123,7 @@ static irqreturn_t da9062_thermal_irq_handler(int irq, void *data)
> static int da9062_thermal_get_mode(struct thermal_zone_device *z,
> enum thermal_device_mode *mode)
> {
> - struct da9062_thermal *thermal = z->devdata;
> - *mode = thermal->mode;
> + *mode = z->mode;
> return 0;
> }
>
> @@ -233,7 +231,6 @@ static int da9062_thermal_probe(struct platform_device *pdev)
>
> thermal->config = match->data;
> thermal->hw = chip;
> - thermal->mode = THERMAL_DEVICE_ENABLED;
> thermal->dev = &pdev->dev;
>
> INIT_DELAYED_WORK(&thermal->work, da9062_thermal_poll_on);
> @@ -248,6 +245,7 @@ static int da9062_thermal_probe(struct platform_device *pdev)
> ret = PTR_ERR(thermal->zone);
> goto err;
> }
> + thermal->zone->mode = THERMAL_DEVICE_ENABLED;
>
> dev_dbg(&pdev->dev,
> "TJUNC temperature polling period set at %d ms\n",
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index e761c9b42217..9a1114d721b6 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -197,7 +197,6 @@ struct imx_thermal_data {
> struct cpufreq_policy *policy;
> struct thermal_zone_device *tz;
> struct thermal_cooling_device *cdev;
> - enum thermal_device_mode mode;
> struct regmap *tempmon;
> u32 c1, c2; /* See formula in imx_init_calib() */
> int temp_passive;
> @@ -256,7 +255,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> bool wait;
> u32 val;
>
> - if (data->mode == THERMAL_DEVICE_ENABLED) {
> + if (tz->mode == THERMAL_DEVICE_ENABLED) {
> /* Check if a measurement is currently in progress */
> regmap_read(map, soc_data->temp_data, &val);
> wait = !(val & soc_data->temp_valid_mask);
> @@ -283,7 +282,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
>
> regmap_read(map, soc_data->temp_data, &val);
>
> - if (data->mode != THERMAL_DEVICE_ENABLED) {
> + if (tz->mode != THERMAL_DEVICE_ENABLED) {
> regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> soc_data->measure_temp_mask);
> regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> @@ -334,9 +333,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> static int imx_get_mode(struct thermal_zone_device *tz,
> enum thermal_device_mode *mode)
> {
> - struct imx_thermal_data *data = tz->devdata;
> -
> - *mode = data->mode;
> + *mode = tz->mode;
>
> return 0;
> }
> @@ -376,7 +373,7 @@ static int imx_set_mode(struct thermal_zone_device *tz,
> }
> }
>
> - data->mode = mode;
> + tz->mode = mode;
> thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>
> return 0;
> @@ -831,7 +828,7 @@ static int imx_thermal_probe(struct platform_device *pdev)
> data->socdata->measure_temp_mask);
>
> data->irq_enabled = true;
> - data->mode = THERMAL_DEVICE_ENABLED;
> + data->tz->mode = THERMAL_DEVICE_ENABLED;
>
> ret = devm_request_threaded_irq(&pdev->dev, data->irq,
> imx_thermal_alarm_irq, imx_thermal_alarm_irq_thread,
> @@ -885,7 +882,7 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
> data->socdata->measure_temp_mask);
> regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> data->socdata->power_down_mask);
> - data->mode = THERMAL_DEVICE_DISABLED;
> + data->tz->mode = THERMAL_DEVICE_DISABLED;
> clk_disable_unprepare(data->thermal_clk);
>
> return 0;
> @@ -905,7 +902,7 @@ static int __maybe_unused imx_thermal_resume(struct device *dev)
> data->socdata->power_down_mask);
> regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> data->socdata->measure_temp_mask);
> - data->mode = THERMAL_DEVICE_ENABLED;
> + data->tz->mode = THERMAL_DEVICE_ENABLED;
>
> return 0;
> }
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index e84faaadff87..f65b2fc09198 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -48,7 +48,6 @@ struct int3400_thermal_priv {
> struct acpi_device *adev;
> struct platform_device *pdev;
> struct thermal_zone_device *thermal;
> - enum thermal_device_mode mode;
> int art_count;
> struct art *arts;
> int trt_count;
> @@ -381,12 +380,7 @@ static int int3400_thermal_get_temp(struct thermal_zone_device *thermal,
> static int int3400_thermal_get_mode(struct thermal_zone_device *thermal,
> enum thermal_device_mode *mode)
> {
> - struct int3400_thermal_priv *priv = thermal->devdata;
> -
> - if (!priv)
> - return -EINVAL;
> -
> - *mode = priv->mode;
> + *mode = thermal->mode;
>
> return 0;
> }
> @@ -404,8 +398,8 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
> mode != THERMAL_DEVICE_DISABLED)
> return -EINVAL;
>
> - if (mode != priv->mode) {
> - priv->mode = mode;
> + if (mode != thermal->mode) {
> + thermal->mode = mode;
> result = int3400_thermal_run_osc(priv->adev->handle,
> priv->current_uuid_index,
> mode == THERMAL_DEVICE_ENABLED);
> diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
> index d704fc104cfd..d77cb3df5ade 100644
> --- a/drivers/thermal/intel/intel_quark_dts_thermal.c
> +++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
> @@ -103,7 +103,6 @@ struct soc_sensor_entry {
> bool locked;
> u32 store_ptps;
> u32 store_dts_enable;
> - enum thermal_device_mode mode;
> struct thermal_zone_device *tzone;
> };
>
> @@ -128,7 +127,7 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
> return ret;
>
> if (out & QRK_DTS_ENABLE_BIT) {
> - aux_entry->mode = THERMAL_DEVICE_ENABLED;
> + tzd->mode = THERMAL_DEVICE_ENABLED;
> return 0;
> }
>
> @@ -139,9 +138,9 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
> if (ret)
> return ret;
>
> - aux_entry->mode = THERMAL_DEVICE_ENABLED;
> + tzd->mode = THERMAL_DEVICE_ENABLED;
> } else {
> - aux_entry->mode = THERMAL_DEVICE_DISABLED;
> + tzd->mode = THERMAL_DEVICE_DISABLED;
> pr_info("DTS is locked. Cannot enable DTS\n");
> ret = -EPERM;
> }
> @@ -161,7 +160,7 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
> return ret;
>
> if (!(out & QRK_DTS_ENABLE_BIT)) {
> - aux_entry->mode = THERMAL_DEVICE_DISABLED;
> + tzd->mode = THERMAL_DEVICE_DISABLED;
> return 0;
> }
>
> @@ -173,9 +172,9 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
> if (ret)
> return ret;
>
> - aux_entry->mode = THERMAL_DEVICE_DISABLED;
> + tzd->mode = THERMAL_DEVICE_DISABLED;
> } else {
> - aux_entry->mode = THERMAL_DEVICE_ENABLED;
> + tzd->mode = THERMAL_DEVICE_ENABLED;
> pr_info("DTS is locked. Cannot disable DTS\n");
> ret = -EPERM;
> }
> @@ -312,8 +311,7 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd,
> static int sys_get_mode(struct thermal_zone_device *tzd,
> enum thermal_device_mode *mode)
> {
> - struct soc_sensor_entry *aux_entry = tzd->devdata;
> - *mode = aux_entry->mode;
> + *mode = tzd->mode;
> return 0;
> }
>
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index ddf88dbe7ba2..c495b1e48ef2 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -51,7 +51,6 @@ struct __thermal_bind_params {
>
> /**
> * struct __thermal_zone - internal representation of a thermal zone
> - * @mode: current thermal zone device mode (enabled/disabled)
> * @passive_delay: polling interval while passive cooling is activated
> * @polling_delay: zone polling interval
> * @slope: slope of the temperature adjustment curve
> @@ -65,7 +64,6 @@ struct __thermal_bind_params {
> */
>
> struct __thermal_zone {
> - enum thermal_device_mode mode;
> int passive_delay;
> int polling_delay;
> int slope;
> @@ -272,9 +270,7 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
> static int of_thermal_get_mode(struct thermal_zone_device *tz,
> enum thermal_device_mode *mode)
> {
> - struct __thermal_zone *data = tz->devdata;
> -
> - *mode = data->mode;
> + *mode = tz->mode;
>
> return 0;
> }
> @@ -296,7 +292,7 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz,
>
> mutex_unlock(&tz->lock);
>
> - data->mode = mode;
> + tz->mode = mode;
> thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>
> return 0;
> @@ -979,7 +975,6 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>
> finish:
> of_node_put(child);
> - tz->mode = THERMAL_DEVICE_DISABLED;
>
> return tz;
>
> @@ -1134,6 +1129,7 @@ int __init of_parse_thermal_zones(void)
> of_thermal_free_zone(tz);
> /* attempting to build remaining zones still */
> }
> + zone->mode = THERMAL_DEVICE_DISABLED;
> }
> of_node_put(np);
>
^ permalink raw reply
* Re: [PATCH v4 04/11] thermal: Store device mode in struct thermal_zone_device
From: Guenter Roeck @ 2020-05-29 19:09 UTC (permalink / raw)
To: Andrzej Pietrasiewicz
Cc: linux-pm, linux-acpi, netdev, linux-wireless, platform-driver-x86,
linux-arm-kernel, linux-renesas-soc, linux-rockchip,
Emmanuel Grumbach, Heiko Stuebner, Rafael J . Wysocki,
Vishal Kulkarni, Luca Coelho, Miquel Raynal, kernel,
Fabio Estevam, Amit Kucheria, Chunyan Zhang, Daniel Lezcano,
Allison Randal, NXP Linux Team
On Thu, May 28, 2020 at 09:20:44PM +0200, Andrzej Pietrasiewicz wrote:
> Prepare for eliminating get_mode().
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
Based on discussion:
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Guenter
> ---
> drivers/acpi/thermal.c | 18 ++++++----------
> .../ethernet/mellanox/mlxsw/core_thermal.c | 21 +++++++------------
> drivers/platform/x86/acerhdf.c | 15 ++++++-------
> drivers/thermal/da9062-thermal.c | 6 ++----
> drivers/thermal/imx_thermal.c | 17 +++++++--------
> .../intel/int340x_thermal/int3400_thermal.c | 12 +++--------
> .../thermal/intel/intel_quark_dts_thermal.c | 16 +++++++-------
> drivers/thermal/thermal_of.c | 10 +++------
> 8 files changed, 44 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index fb46070c66d8..4ba273f49d87 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -172,7 +172,6 @@ struct acpi_thermal {
> struct acpi_thermal_trips trips;
> struct acpi_handle_list devices;
> struct thermal_zone_device *thermal_zone;
> - enum thermal_device_mode mode;
> int kelvin_offset; /* in millidegrees */
> struct work_struct thermal_check_work;
> };
> @@ -500,7 +499,7 @@ static void acpi_thermal_check(void *data)
> {
> struct acpi_thermal *tz = data;
>
> - if (tz->mode != THERMAL_DEVICE_ENABLED)
> + if (tz->thermal_zone->mode != THERMAL_DEVICE_ENABLED)
> return;
>
> thermal_zone_device_update(tz->thermal_zone,
> @@ -529,12 +528,7 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
> static int thermal_get_mode(struct thermal_zone_device *thermal,
> enum thermal_device_mode *mode)
> {
> - struct acpi_thermal *tz = thermal->devdata;
> -
> - if (!tz)
> - return -EINVAL;
> -
> - *mode = tz->mode;
> + *mode = thermal->mode;
>
> return 0;
> }
> @@ -556,11 +550,11 @@ static int thermal_set_mode(struct thermal_zone_device *thermal,
> if (mode == THERMAL_DEVICE_DISABLED)
> pr_warn("thermal zone will be disabled\n");
>
> - if (mode != tz->mode) {
> - tz->mode = mode;
> + if (mode != tz->thermal_zone->mode) {
> + tz->thermal_zone->mode = mode;
> ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> "%s kernel ACPI thermal control\n",
> - tz->mode == THERMAL_DEVICE_ENABLED ?
> + tz->thermal_zone->mode == THERMAL_DEVICE_ENABLED ?
> "Enable" : "Disable"));
> acpi_thermal_check(tz);
> }
> @@ -912,7 +906,7 @@ static int acpi_thermal_register_thermal_zone(struct acpi_thermal *tz)
> goto remove_dev_link;
> }
>
> - tz->mode = THERMAL_DEVICE_ENABLED;
> + tz->thermal_zone->mode = THERMAL_DEVICE_ENABLED;
>
> dev_info(&tz->device->dev, "registered as thermal_zone%d\n",
> tz->thermal_zone->id);
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> index ce0a6837daa3..aa082e8a0b13 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
> @@ -98,7 +98,6 @@ struct mlxsw_thermal_module {
> struct mlxsw_thermal *parent;
> struct thermal_zone_device *tzdev;
> struct mlxsw_thermal_trip trips[MLXSW_THERMAL_NUM_TRIPS];
> - enum thermal_device_mode mode;
> int module; /* Module or gearbox number */
> };
>
> @@ -110,7 +109,6 @@ struct mlxsw_thermal {
> struct thermal_cooling_device *cdevs[MLXSW_MFCR_PWMS_MAX];
> u8 cooling_levels[MLXSW_THERMAL_MAX_STATE + 1];
> struct mlxsw_thermal_trip trips[MLXSW_THERMAL_NUM_TRIPS];
> - enum thermal_device_mode mode;
> struct mlxsw_thermal_module *tz_module_arr;
> u8 tz_module_num;
> struct mlxsw_thermal_module *tz_gearbox_arr;
> @@ -280,9 +278,7 @@ static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
> static int mlxsw_thermal_get_mode(struct thermal_zone_device *tzdev,
> enum thermal_device_mode *mode)
> {
> - struct mlxsw_thermal *thermal = tzdev->devdata;
> -
> - *mode = thermal->mode;
> + *mode = tzdev->mode;
>
> return 0;
> }
> @@ -299,9 +295,9 @@ static int mlxsw_thermal_set_mode(struct thermal_zone_device *tzdev,
> else
> tzdev->polling_delay = 0;
>
> + tzdev->mode = mode;
> mutex_unlock(&tzdev->lock);
>
> - thermal->mode = mode;
> thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
>
> return 0;
> @@ -469,9 +465,7 @@ static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
> static int mlxsw_thermal_module_mode_get(struct thermal_zone_device *tzdev,
> enum thermal_device_mode *mode)
> {
> - struct mlxsw_thermal_module *tz = tzdev->devdata;
> -
> - *mode = tz->mode;
> + *mode = tzdev->mode;
>
> return 0;
> }
> @@ -489,9 +483,10 @@ static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
> else
> tzdev->polling_delay = 0;
>
> + tzdev->mode = mode;
> +
> mutex_unlock(&tzdev->lock);
>
> - tz->mode = mode;
> thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
>
> return 0;
> @@ -765,7 +760,7 @@ mlxsw_thermal_module_tz_init(struct mlxsw_thermal_module *module_tz)
> return err;
> }
>
> - module_tz->mode = THERMAL_DEVICE_ENABLED;
> + module_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
> return 0;
> }
>
> @@ -881,7 +876,7 @@ mlxsw_thermal_gearbox_tz_init(struct mlxsw_thermal_module *gearbox_tz)
> if (IS_ERR(gearbox_tz->tzdev))
> return PTR_ERR(gearbox_tz->tzdev);
>
> - gearbox_tz->mode = THERMAL_DEVICE_ENABLED;
> + gearbox_tz->tzdev->mode = THERMAL_DEVICE_ENABLED;
> return 0;
> }
>
> @@ -1050,7 +1045,7 @@ int mlxsw_thermal_init(struct mlxsw_core *core,
> if (err)
> goto err_unreg_modules_tzdev;
>
> - thermal->mode = THERMAL_DEVICE_ENABLED;
> + thermal->tzdev->mode = THERMAL_DEVICE_ENABLED;
> *p_thermal = thermal;
> return 0;
>
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 830a8b060e74..97b288485837 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -68,7 +68,6 @@ static int kernelmode = 1;
> #else
> static int kernelmode;
> #endif
> -static enum thermal_device_mode thermal_mode;
>
> static unsigned int interval = 10;
> static unsigned int fanon = 60000;
> @@ -398,15 +397,16 @@ static inline void acerhdf_revert_to_bios_mode(void)
> {
> acerhdf_change_fanstate(ACERHDF_FAN_AUTO);
> kernelmode = 0;
> - thermal_mode = THERMAL_DEVICE_DISABLED;
> - if (thz_dev)
> + if (thz_dev) {
> + thz_dev->mode = THERMAL_DEVICE_DISABLED;
> thz_dev->polling_delay = 0;
> + }
> pr_notice("kernel mode fan control OFF\n");
> }
> static inline void acerhdf_enable_kernelmode(void)
> {
> kernelmode = 1;
> - thermal_mode = THERMAL_DEVICE_ENABLED;
> + thz_dev->mode = THERMAL_DEVICE_ENABLED;
>
> thz_dev->polling_delay = interval*1000;
> thermal_zone_device_update(thz_dev, THERMAL_EVENT_UNSPECIFIED);
> @@ -419,7 +419,7 @@ static int acerhdf_get_mode(struct thermal_zone_device *thermal,
> if (verbose)
> pr_notice("kernel mode fan control %d\n", kernelmode);
>
> - *mode = thermal_mode;
> + *mode = thermal->mode;
>
> return 0;
> }
> @@ -741,8 +741,6 @@ static int __init acerhdf_register_thermal(void)
> if (IS_ERR(cl_dev))
> return -EINVAL;
>
> - thermal_mode = kernelmode ?
> - THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED;
> thz_dev = thermal_zone_device_register("acerhdf", 2, 0, NULL,
> &acerhdf_dev_ops,
> &acerhdf_zone_params, 0,
> @@ -750,6 +748,9 @@ static int __init acerhdf_register_thermal(void)
> if (IS_ERR(thz_dev))
> return -EINVAL;
>
> + thz_dev->mode = kernelmode ?
> + THERMAL_DEVICE_ENABLED : THERMAL_DEVICE_DISABLED;
> +
> if (strcmp(thz_dev->governor->name,
> acerhdf_zone_params.governor_name)) {
> pr_err("Didn't get thermal governor %s, perhaps not compiled into thermal subsystem.\n",
> diff --git a/drivers/thermal/da9062-thermal.c b/drivers/thermal/da9062-thermal.c
> index c32709badeda..a14c7981c7c7 100644
> --- a/drivers/thermal/da9062-thermal.c
> +++ b/drivers/thermal/da9062-thermal.c
> @@ -49,7 +49,6 @@ struct da9062_thermal {
> struct da9062 *hw;
> struct delayed_work work;
> struct thermal_zone_device *zone;
> - enum thermal_device_mode mode;
> struct mutex lock; /* protection for da9062_thermal temperature */
> int temperature;
> int irq;
> @@ -124,8 +123,7 @@ static irqreturn_t da9062_thermal_irq_handler(int irq, void *data)
> static int da9062_thermal_get_mode(struct thermal_zone_device *z,
> enum thermal_device_mode *mode)
> {
> - struct da9062_thermal *thermal = z->devdata;
> - *mode = thermal->mode;
> + *mode = z->mode;
> return 0;
> }
>
> @@ -233,7 +231,6 @@ static int da9062_thermal_probe(struct platform_device *pdev)
>
> thermal->config = match->data;
> thermal->hw = chip;
> - thermal->mode = THERMAL_DEVICE_ENABLED;
> thermal->dev = &pdev->dev;
>
> INIT_DELAYED_WORK(&thermal->work, da9062_thermal_poll_on);
> @@ -248,6 +245,7 @@ static int da9062_thermal_probe(struct platform_device *pdev)
> ret = PTR_ERR(thermal->zone);
> goto err;
> }
> + thermal->zone->mode = THERMAL_DEVICE_ENABLED;
>
> dev_dbg(&pdev->dev,
> "TJUNC temperature polling period set at %d ms\n",
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index e761c9b42217..9a1114d721b6 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -197,7 +197,6 @@ struct imx_thermal_data {
> struct cpufreq_policy *policy;
> struct thermal_zone_device *tz;
> struct thermal_cooling_device *cdev;
> - enum thermal_device_mode mode;
> struct regmap *tempmon;
> u32 c1, c2; /* See formula in imx_init_calib() */
> int temp_passive;
> @@ -256,7 +255,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> bool wait;
> u32 val;
>
> - if (data->mode == THERMAL_DEVICE_ENABLED) {
> + if (tz->mode == THERMAL_DEVICE_ENABLED) {
> /* Check if a measurement is currently in progress */
> regmap_read(map, soc_data->temp_data, &val);
> wait = !(val & soc_data->temp_valid_mask);
> @@ -283,7 +282,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
>
> regmap_read(map, soc_data->temp_data, &val);
>
> - if (data->mode != THERMAL_DEVICE_ENABLED) {
> + if (tz->mode != THERMAL_DEVICE_ENABLED) {
> regmap_write(map, soc_data->sensor_ctrl + REG_CLR,
> soc_data->measure_temp_mask);
> regmap_write(map, soc_data->sensor_ctrl + REG_SET,
> @@ -334,9 +333,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
> static int imx_get_mode(struct thermal_zone_device *tz,
> enum thermal_device_mode *mode)
> {
> - struct imx_thermal_data *data = tz->devdata;
> -
> - *mode = data->mode;
> + *mode = tz->mode;
>
> return 0;
> }
> @@ -376,7 +373,7 @@ static int imx_set_mode(struct thermal_zone_device *tz,
> }
> }
>
> - data->mode = mode;
> + tz->mode = mode;
> thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>
> return 0;
> @@ -831,7 +828,7 @@ static int imx_thermal_probe(struct platform_device *pdev)
> data->socdata->measure_temp_mask);
>
> data->irq_enabled = true;
> - data->mode = THERMAL_DEVICE_ENABLED;
> + data->tz->mode = THERMAL_DEVICE_ENABLED;
>
> ret = devm_request_threaded_irq(&pdev->dev, data->irq,
> imx_thermal_alarm_irq, imx_thermal_alarm_irq_thread,
> @@ -885,7 +882,7 @@ static int __maybe_unused imx_thermal_suspend(struct device *dev)
> data->socdata->measure_temp_mask);
> regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> data->socdata->power_down_mask);
> - data->mode = THERMAL_DEVICE_DISABLED;
> + data->tz->mode = THERMAL_DEVICE_DISABLED;
> clk_disable_unprepare(data->thermal_clk);
>
> return 0;
> @@ -905,7 +902,7 @@ static int __maybe_unused imx_thermal_resume(struct device *dev)
> data->socdata->power_down_mask);
> regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
> data->socdata->measure_temp_mask);
> - data->mode = THERMAL_DEVICE_ENABLED;
> + data->tz->mode = THERMAL_DEVICE_ENABLED;
>
> return 0;
> }
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index e84faaadff87..f65b2fc09198 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -48,7 +48,6 @@ struct int3400_thermal_priv {
> struct acpi_device *adev;
> struct platform_device *pdev;
> struct thermal_zone_device *thermal;
> - enum thermal_device_mode mode;
> int art_count;
> struct art *arts;
> int trt_count;
> @@ -381,12 +380,7 @@ static int int3400_thermal_get_temp(struct thermal_zone_device *thermal,
> static int int3400_thermal_get_mode(struct thermal_zone_device *thermal,
> enum thermal_device_mode *mode)
> {
> - struct int3400_thermal_priv *priv = thermal->devdata;
> -
> - if (!priv)
> - return -EINVAL;
> -
> - *mode = priv->mode;
> + *mode = thermal->mode;
>
> return 0;
> }
> @@ -404,8 +398,8 @@ static int int3400_thermal_set_mode(struct thermal_zone_device *thermal,
> mode != THERMAL_DEVICE_DISABLED)
> return -EINVAL;
>
> - if (mode != priv->mode) {
> - priv->mode = mode;
> + if (mode != thermal->mode) {
> + thermal->mode = mode;
> result = int3400_thermal_run_osc(priv->adev->handle,
> priv->current_uuid_index,
> mode == THERMAL_DEVICE_ENABLED);
> diff --git a/drivers/thermal/intel/intel_quark_dts_thermal.c b/drivers/thermal/intel/intel_quark_dts_thermal.c
> index d704fc104cfd..d77cb3df5ade 100644
> --- a/drivers/thermal/intel/intel_quark_dts_thermal.c
> +++ b/drivers/thermal/intel/intel_quark_dts_thermal.c
> @@ -103,7 +103,6 @@ struct soc_sensor_entry {
> bool locked;
> u32 store_ptps;
> u32 store_dts_enable;
> - enum thermal_device_mode mode;
> struct thermal_zone_device *tzone;
> };
>
> @@ -128,7 +127,7 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
> return ret;
>
> if (out & QRK_DTS_ENABLE_BIT) {
> - aux_entry->mode = THERMAL_DEVICE_ENABLED;
> + tzd->mode = THERMAL_DEVICE_ENABLED;
> return 0;
> }
>
> @@ -139,9 +138,9 @@ static int soc_dts_enable(struct thermal_zone_device *tzd)
> if (ret)
> return ret;
>
> - aux_entry->mode = THERMAL_DEVICE_ENABLED;
> + tzd->mode = THERMAL_DEVICE_ENABLED;
> } else {
> - aux_entry->mode = THERMAL_DEVICE_DISABLED;
> + tzd->mode = THERMAL_DEVICE_DISABLED;
> pr_info("DTS is locked. Cannot enable DTS\n");
> ret = -EPERM;
> }
> @@ -161,7 +160,7 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
> return ret;
>
> if (!(out & QRK_DTS_ENABLE_BIT)) {
> - aux_entry->mode = THERMAL_DEVICE_DISABLED;
> + tzd->mode = THERMAL_DEVICE_DISABLED;
> return 0;
> }
>
> @@ -173,9 +172,9 @@ static int soc_dts_disable(struct thermal_zone_device *tzd)
> if (ret)
> return ret;
>
> - aux_entry->mode = THERMAL_DEVICE_DISABLED;
> + tzd->mode = THERMAL_DEVICE_DISABLED;
> } else {
> - aux_entry->mode = THERMAL_DEVICE_ENABLED;
> + tzd->mode = THERMAL_DEVICE_ENABLED;
> pr_info("DTS is locked. Cannot disable DTS\n");
> ret = -EPERM;
> }
> @@ -312,8 +311,7 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd,
> static int sys_get_mode(struct thermal_zone_device *tzd,
> enum thermal_device_mode *mode)
> {
> - struct soc_sensor_entry *aux_entry = tzd->devdata;
> - *mode = aux_entry->mode;
> + *mode = tzd->mode;
> return 0;
> }
>
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index ddf88dbe7ba2..c495b1e48ef2 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -51,7 +51,6 @@ struct __thermal_bind_params {
>
> /**
> * struct __thermal_zone - internal representation of a thermal zone
> - * @mode: current thermal zone device mode (enabled/disabled)
> * @passive_delay: polling interval while passive cooling is activated
> * @polling_delay: zone polling interval
> * @slope: slope of the temperature adjustment curve
> @@ -65,7 +64,6 @@ struct __thermal_bind_params {
> */
>
> struct __thermal_zone {
> - enum thermal_device_mode mode;
> int passive_delay;
> int polling_delay;
> int slope;
> @@ -272,9 +270,7 @@ static int of_thermal_unbind(struct thermal_zone_device *thermal,
> static int of_thermal_get_mode(struct thermal_zone_device *tz,
> enum thermal_device_mode *mode)
> {
> - struct __thermal_zone *data = tz->devdata;
> -
> - *mode = data->mode;
> + *mode = tz->mode;
>
> return 0;
> }
> @@ -296,7 +292,7 @@ static int of_thermal_set_mode(struct thermal_zone_device *tz,
>
> mutex_unlock(&tz->lock);
>
> - data->mode = mode;
> + tz->mode = mode;
> thermal_zone_device_update(tz, THERMAL_EVENT_UNSPECIFIED);
>
> return 0;
> @@ -979,7 +975,6 @@ __init *thermal_of_build_thermal_zone(struct device_node *np)
>
> finish:
> of_node_put(child);
> - tz->mode = THERMAL_DEVICE_DISABLED;
>
> return tz;
>
> @@ -1134,6 +1129,7 @@ int __init of_parse_thermal_zones(void)
> of_thermal_free_zone(tz);
> /* attempting to build remaining zones still */
> }
> + zone->mode = THERMAL_DEVICE_DISABLED;
> }
> of_node_put(np);
>
^ permalink raw reply
* Re: [PATCH v4 04/11] thermal: Store device mode in struct thermal_zone_device
From: Guenter Roeck @ 2020-05-29 19:08 UTC (permalink / raw)
To: Andrzej Pietrasiewicz
Cc: Emmanuel Grumbach, Heiko Stuebner, Kalle Valo, linux-wireless,
Peter Kaestle, platform-driver-x86, Vishal Kulkarni, Luca Coelho,
Miquel Raynal, Shawn Guo, kernel, Fabio Estevam, Amit Kucheria,
linux-rockchip, Chunyan Zhang, Daniel Lezcano, linux-acpi,
linux-arm-kernel, Darren Hart, Zhang Rui, Gayatri Kammela,
NXP Linux Team, Johannes Berg, linux-pm, Sascha Hauer,
Intel Linux Wireless, Ido Schimmel, Niklas Söderlund,
Jiri Pirko, Orson Zhai, Thomas Gleixner, Allison Randal,
Support Opensource, netdev, Rafael J . Wysocki, Sebastian Reichel,
linux-renesas-soc, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Baolin Wang, Len Brown, Enrico Weigelt,
David S . Miller, Andy Shevchenko
In-Reply-To: <a0c0310f-9870-47be-4ca3-c07e41c380fc@collabora.com>
On 5/29/20 10:21 AM, Andrzej Pietrasiewicz wrote:
> Hi again,
>
> W dniu 29.05.2020 o 18:08, Andrzej Pietrasiewicz pisze:
>> Hi Guenter,
>>
>> W dniu 29.05.2020 o 17:42, Guenter Roeck pisze:
>>> On Thu, May 28, 2020 at 09:20:44PM +0200, Andrzej Pietrasiewicz wrote:
>>>> Prepare for eliminating get_mode().
>>>>
>>> Might be worthwhile to explain (not only in the subject) what you are
>>> doing here.
>>>
>>>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>>>> ---
>>>> drivers/acpi/thermal.c | 18 ++++++----------
>>>> .../ethernet/mellanox/mlxsw/core_thermal.c | 21 +++++++------------
>>>> drivers/platform/x86/acerhdf.c | 15 ++++++-------
>>>> drivers/thermal/da9062-thermal.c | 6 ++----
>>>> drivers/thermal/imx_thermal.c | 17 +++++++--------
>>>> .../intel/int340x_thermal/int3400_thermal.c | 12 +++--------
>>>> .../thermal/intel/intel_quark_dts_thermal.c | 16 +++++++-------
>>>> drivers/thermal/thermal_of.c | 10 +++------
>>>
>>> After this patch is applied on top of the thermal 'testing' branch,
>>> there are still local instances of thermal_device_mode in
>>> drivers/thermal/st/stm_thermal.c
>>> drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>>>
>>> If there is a reason not to replace those, it might make sense to explain
>>> it here.
>>>
>>
>> My understanding is that these two are sensor devices which are "plugged"
>> into their "parent" thermal zone device. The latter is the "proper" tzd.
>> They both use thermal_zone_of_device_ops instead of thermal_zone_device_ops.
>> The former doesn't even have get_mode(). The thermal core, when it calls
>> get_mode(), operates on the "parent" thermal zone devices.
>>
>> Consequently, the drivers you mention use their "mode" members for
>> their private purpose, not for the purpose of storing the "parent"
>> thermal zone device mode.
>>
>
> Let me also say it differently.
>
> Both drivers which you mention use devm_thermal_zone_of_sensor_register().
> It calls thermal_zone_of_sensor_register(), which "will search the list of
> thermal zones described in device tree and look for the zone that refer to
> the sensor device pointed by @dev->of_node as temperature providers. For
> the zone pointing to the sensor node, the sensor will be added to the DT
> thermal zone device." When a match is found thermal_zone_of_add_sensor()
> is invoked, which (using thermal_zone_get_zone_by_name()) iterates over
> all registered thermal_zone_devices. The one eventually found will be
> returned and propagated to the original caller of
> devm_thermal_zone_of_sensor_register(). The state of this returned
> device is managed elsewhere (in that device's struct tzd). The "mode"
> member you are referring to is thus unrelated.
>
Quite confusing, especially since the ti-soc driver doesn't seem to use
the variable at all after setting it, and the stm_thermal driver uses it
to reflect power status associated with suspend/resume. So, yes, I agree
this is fine.
Thanks,
Guenter
> Regards,
>
> Andrzej
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 04/11] thermal: Store device mode in struct thermal_zone_device
From: Guenter Roeck @ 2020-05-29 19:08 UTC (permalink / raw)
To: Andrzej Pietrasiewicz
Cc: linux-pm, linux-acpi, netdev, linux-wireless, platform-driver-x86,
linux-arm-kernel, linux-renesas-soc, linux-rockchip,
Emmanuel Grumbach, Heiko Stuebner, Rafael J . Wysocki,
Vishal Kulkarni, Luca Coelho, Miquel Raynal, kernel,
Fabio Estevam, Amit Kucheria, Chunyan Zhang, Daniel Lezcano,
Allison Randal, NXP Linux Team, Darren Hart, Zhang Rui,
Gayatri Kammela, Len Brown, Johannes Berg, Intel Linux Wireless,
Sascha Hauer, Ido Schimmel, Baolin Wang, Jiri Pirko, Orson Zhai,
Thomas Gleixner, Kalle Valo, Support Opensource, Enrico Weigelt,
Peter Kaestle, Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Niklas Söderlund, Shawn Guo,
David S . Miller, Andy Shevchenko
In-Reply-To: <a0c0310f-9870-47be-4ca3-c07e41c380fc@collabora.com>
On 5/29/20 10:21 AM, Andrzej Pietrasiewicz wrote:
> Hi again,
>
> W dniu 29.05.2020 o 18:08, Andrzej Pietrasiewicz pisze:
>> Hi Guenter,
>>
>> W dniu 29.05.2020 o 17:42, Guenter Roeck pisze:
>>> On Thu, May 28, 2020 at 09:20:44PM +0200, Andrzej Pietrasiewicz wrote:
>>>> Prepare for eliminating get_mode().
>>>>
>>> Might be worthwhile to explain (not only in the subject) what you are
>>> doing here.
>>>
>>>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>>>> ---
>>>> drivers/acpi/thermal.c | 18 ++++++----------
>>>> .../ethernet/mellanox/mlxsw/core_thermal.c | 21 +++++++------------
>>>> drivers/platform/x86/acerhdf.c | 15 ++++++-------
>>>> drivers/thermal/da9062-thermal.c | 6 ++----
>>>> drivers/thermal/imx_thermal.c | 17 +++++++--------
>>>> .../intel/int340x_thermal/int3400_thermal.c | 12 +++--------
>>>> .../thermal/intel/intel_quark_dts_thermal.c | 16 +++++++-------
>>>> drivers/thermal/thermal_of.c | 10 +++------
>>>
>>> After this patch is applied on top of the thermal 'testing' branch,
>>> there are still local instances of thermal_device_mode in
>>> drivers/thermal/st/stm_thermal.c
>>> drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>>>
>>> If there is a reason not to replace those, it might make sense to explain
>>> it here.
>>>
>>
>> My understanding is that these two are sensor devices which are "plugged"
>> into their "parent" thermal zone device. The latter is the "proper" tzd.
>> They both use thermal_zone_of_device_ops instead of thermal_zone_device_ops.
>> The former doesn't even have get_mode(). The thermal core, when it calls
>> get_mode(), operates on the "parent" thermal zone devices.
>>
>> Consequently, the drivers you mention use their "mode" members for
>> their private purpose, not for the purpose of storing the "parent"
>> thermal zone device mode.
>>
>
> Let me also say it differently.
>
> Both drivers which you mention use devm_thermal_zone_of_sensor_register().
> It calls thermal_zone_of_sensor_register(), which "will search the list of
> thermal zones described in device tree and look for the zone that refer to
> the sensor device pointed by @dev->of_node as temperature providers. For
> the zone pointing to the sensor node, the sensor will be added to the DT
> thermal zone device." When a match is found thermal_zone_of_add_sensor()
> is invoked, which (using thermal_zone_get_zone_by_name()) iterates over
> all registered thermal_zone_devices. The one eventually found will be
> returned and propagated to the original caller of
> devm_thermal_zone_of_sensor_register(). The state of this returned
> device is managed elsewhere (in that device's struct tzd). The "mode"
> member you are referring to is thus unrelated.
>
Quite confusing, especially since the ti-soc driver doesn't seem to use
the variable at all after setting it, and the stm_thermal driver uses it
to reflect power status associated with suspend/resume. So, yes, I agree
this is fine.
Thanks,
Guenter
> Regards,
>
> Andrzej
^ permalink raw reply
* Re: [PATCH v4 04/11] thermal: Store device mode in struct thermal_zone_device
From: Guenter Roeck @ 2020-05-29 19:08 UTC (permalink / raw)
To: Andrzej Pietrasiewicz
Cc: linux-pm, linux-acpi, netdev, linux-wireless, platform-driver-x86,
linux-arm-kernel, linux-renesas-soc, linux-rockchip,
Emmanuel Grumbach, Heiko Stuebner, Rafael J . Wysocki,
Vishal Kulkarni, Luca Coelho, Miquel Raynal, kernel,
Fabio Estevam, Amit Kucheria, Chunyan Zhang, Daniel Lezcano,
Allison Randal, NXP Linux Team
In-Reply-To: <a0c0310f-9870-47be-4ca3-c07e41c380fc@collabora.com>
On 5/29/20 10:21 AM, Andrzej Pietrasiewicz wrote:
> Hi again,
>
> W dniu 29.05.2020 o 18:08, Andrzej Pietrasiewicz pisze:
>> Hi Guenter,
>>
>> W dniu 29.05.2020 o 17:42, Guenter Roeck pisze:
>>> On Thu, May 28, 2020 at 09:20:44PM +0200, Andrzej Pietrasiewicz wrote:
>>>> Prepare for eliminating get_mode().
>>>>
>>> Might be worthwhile to explain (not only in the subject) what you are
>>> doing here.
>>>
>>>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
>>>> ---
>>>> drivers/acpi/thermal.c | 18 ++++++----------
>>>> .../ethernet/mellanox/mlxsw/core_thermal.c | 21 +++++++------------
>>>> drivers/platform/x86/acerhdf.c | 15 ++++++-------
>>>> drivers/thermal/da9062-thermal.c | 6 ++----
>>>> drivers/thermal/imx_thermal.c | 17 +++++++--------
>>>> .../intel/int340x_thermal/int3400_thermal.c | 12 +++--------
>>>> .../thermal/intel/intel_quark_dts_thermal.c | 16 +++++++-------
>>>> drivers/thermal/thermal_of.c | 10 +++------
>>>
>>> After this patch is applied on top of the thermal 'testing' branch,
>>> there are still local instances of thermal_device_mode in
>>> drivers/thermal/st/stm_thermal.c
>>> drivers/thermal/ti-soc-thermal/ti-thermal-common.c
>>>
>>> If there is a reason not to replace those, it might make sense to explain
>>> it here.
>>>
>>
>> My understanding is that these two are sensor devices which are "plugged"
>> into their "parent" thermal zone device. The latter is the "proper" tzd.
>> They both use thermal_zone_of_device_ops instead of thermal_zone_device_ops.
>> The former doesn't even have get_mode(). The thermal core, when it calls
>> get_mode(), operates on the "parent" thermal zone devices.
>>
>> Consequently, the drivers you mention use their "mode" members for
>> their private purpose, not for the purpose of storing the "parent"
>> thermal zone device mode.
>>
>
> Let me also say it differently.
>
> Both drivers which you mention use devm_thermal_zone_of_sensor_register().
> It calls thermal_zone_of_sensor_register(), which "will search the list of
> thermal zones described in device tree and look for the zone that refer to
> the sensor device pointed by @dev->of_node as temperature providers. For
> the zone pointing to the sensor node, the sensor will be added to the DT
> thermal zone device." When a match is found thermal_zone_of_add_sensor()
> is invoked, which (using thermal_zone_get_zone_by_name()) iterates over
> all registered thermal_zone_devices. The one eventually found will be
> returned and propagated to the original caller of
> devm_thermal_zone_of_sensor_register(). The state of this returned
> device is managed elsewhere (in that device's struct tzd). The "mode"
> member you are referring to is thus unrelated.
>
Quite confusing, especially since the ti-soc driver doesn't seem to use
the variable at all after setting it, and the stm_thermal driver uses it
to reflect power status associated with suspend/resume. So, yes, I agree
this is fine.
Thanks,
Guenter
> Regards,
>
> Andrzej
^ permalink raw reply
* Re: Git fetches whole repository and not just latest
From: Andy Shevchenko @ 2020-05-29 19:08 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Konstantin Ryabitsev
In-Reply-To: <20200529172955.GA123244@google.com>
On Fri, May 29, 2020 at 8:29 PM Jonathan Nieder <jrnieder@gmail.com> wrote:
> Andy Shevchenko wrote:
>
> > So, I have a local repository which has many remotes added (~20 or
> > so). Most of them are from the same Git server, but few are from
> > different servers.
> [...]
> > origin, for example, almost everyday now gets in full (1.21 GiB!),
> > while others have no pattern.
> >
> > I would like to know if it's problem of proxy, or that specific Git
> > server or is it (new) bug in Git?
>
> What Git version are you using?
git version 2.26.2
> Can you test 2.27.0-rc2?
Is there any deb package?
> I believe this is fixed by
>
> commit 2f0a093dd640e0dad0b261dae2427f2541b5426c
> Author: Jonathan Tan <jonathantanmy@google.com>
> Date: Mon Apr 27 17:01:10 2020 -0700
>
> fetch-pack: in protocol v2, reset in_vain upon ACK
>
> which is part of 2.27.0-rc0.
>
> Thanks and hope that helps,
I'll try to allocate slot to test later, if there is deb available, I
can do it sooner.
Thanks!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] usb: dwc2: Fix shutdown callback in platform
From: Doug Anderson @ 2020-05-29 19:07 UTC (permalink / raw)
To: Alan Stern
Cc: Frank Mori Hess, Minas Harutyunyan, John Youn, Felipe Balbi,
Greg Kroah-Hartman, linux-usb@vger.kernel.org, # 4.0+
In-Reply-To: <20200529190031.GA2271@rowland.harvard.edu>
Hi,
On Fri, May 29, 2020 at 12:00 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Fri, May 29, 2020 at 11:45:53AM -0700, Doug Anderson wrote:
> > Hi,
> >
> > On Fri, May 29, 2020 at 11:21 AM Frank Mori Hess <fmh6jj@gmail.com> wrote:
> > >
> > > On Fri, May 29, 2020 at 1:53 PM Doug Anderson <dianders@chromium.org> wrote:
> > > > >
> > > > > I don't get it. A hypothetical machine could have literally anything
> > > > > sharing the IRQ line, right?
> > > >
> > > > It's not a real physical line, though? I don't think it's common to
> > > > have a shared interrupt between different IP blocks in a given SoC.
> > > > Even if it existed, all the drivers should disable their interrupts?
> > >
> > > I don't know, it's a hypothetical machine so it can be whatever you
> > > want. The driver requests shared irqs, if it doesn't actually support
> > > irq sharing, it shouldn't request them.
> >
> > I guess? As I understood it drivers have to be very carefully coded
> > up to support sharing their IRQ with someone else and I'm not
> > convinced dwc2 does that anyway. Certainly it doesn't hurt to keep
> > dwc2 clean, but until I see someone that's actually sharing dwc2's
> > interrupt and I can actually see an example I'm not sure I'm going to
> > spend too much time thinking about it.
>
> This is silly. If the driver says it supports shared IRQs, then it
> should actually support them.
Ah. The IRQ here is "shared" because of the way that the dwc2 driver
is architected. The "gadget" mode and "host" mode driver "share" the
IRQ. ...but there is no non-dwc2 device sharing. In other words,
it's not like there's a UART or and i2c device that would share it.
> > > > > Anyways, my screaming interrupt occurs after a a new kernel has been
> > > > > booted with kexec. In this case, it doesn't matter if the old kernel
> > > > > called disable_irq or not. As soon as the new kernel re-enables the
> > > > > interrupt line, the kernel immediately disables it again with a
> > > > > backtrace due to the unhandled screaming interrupt. That's why the
> > > > > dwc2 hardware needs to have its interrupts turned off when the old
> > > > > kernel is shutdown.
> > > >
> > > > Isn't that a bug with your new kernel? I've seen plenty of bugs where
> > > > drivers enable their interrupt before their interrupt handler is set
> > > > to handle it. You never know what state the bootloader (or previous
> > > > kernel) might have left things in and if an interrupt was pending it
> > > > shouldn't kill you.
> > >
> > > It wouldn't hurt to add disabling of the dwc2 irq early in dwc2
> > > initialization,
> >
> > More than it not hurting, I'd consider it a bug in the driver (and a
> > much more serious one than shutdown not disabling the interrupt).
>
> Normally the first thing a driver would do is reset the hardware, and
> that reset should disable any interrupt source.
Yup.
> > > but why leave the irq screaming after shutdown?
> >
> > Sure. So I guess the answer is to just do both disable the interrupt
> > and make sure that the interrupt handler has finished.
> >
> > dwc2_disable_global_interrupts(hsotg);
> > disable_irq(hsotg->irq);
>
> Drivers with shared IRQs don't call disable_irq(); they call
> synchronize_irq(). It will do what you want (wait until all running
> handlers have returned).
OK.
-Doug
^ permalink raw reply
* Re: [PATCH v3 00/28] KVM: nSVM: event fixes and migration support
From: Paolo Bonzini @ 2020-05-29 19:07 UTC (permalink / raw)
To: Sean Christopherson; +Cc: linux-kernel, kvm
In-Reply-To: <20200529175939.GC1074@linux.intel.com>
On 29/05/20 19:59, Sean Christopherson wrote:
>> [PATCH v3 00/28] KVM: nSVM: event fixes and migration support
> You've got something funky going on with the way you generate cover letters,
> looks like it doesn't count patches authored by someone else. The 'v3' is
> also missing from the patches, though I suppose some heathens do that on
> purpose.
No, I simply had a draft of the cover letter ready, and when I decided
to include Vitaly's patches I didn't update the subject.
Paolo
^ permalink raw reply
* Re: [PATCH v14 1/3] dt-bindings: i2c: npcm7xx: add NPCM I2C controller
From: Rob Herring @ 2020-05-29 19:07 UTC (permalink / raw)
To: Tali Perry
Cc: devicetree, benjaminfair, kfting, avifishman70, venture, openbmc,
wsa, brendanhiggins, ofery, linux-kernel, yuenn, robh+dt,
linux-i2c, andriy.shevchenko, linux-arm-kernel, tmaimon77
In-Reply-To: <20200527200820.47359-2-tali.perry1@gmail.com>
On Wed, 27 May 2020 23:08:18 +0300, Tali Perry wrote:
> Added device tree binding documentation for Nuvoton BMC
> NPCM I2C controller.
>
> Signed-off-by: Tali Perry <tali.perry1@gmail.com>
> ---
> .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml | 62 +++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v14 1/3] dt-bindings: i2c: npcm7xx: add NPCM I2C controller
From: Rob Herring @ 2020-05-29 19:07 UTC (permalink / raw)
To: Tali Perry
Cc: venture, yuenn, andriy.shevchenko, linux-arm-kernel, ofery,
openbmc, brendanhiggins, benjaminfair, linux-kernel, linux-i2c,
kfting, devicetree, robh+dt, avifishman70, wsa, tmaimon77
In-Reply-To: <20200527200820.47359-2-tali.perry1@gmail.com>
On Wed, 27 May 2020 23:08:18 +0300, Tali Perry wrote:
> Added device tree binding documentation for Nuvoton BMC
> NPCM I2C controller.
>
> Signed-off-by: Tali Perry <tali.perry1@gmail.com>
> ---
> .../bindings/i2c/nuvoton,npcm7xx-i2c.yaml | 62 +++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i2c/nuvoton,npcm7xx-i2c.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [Buildroot] [PATCH v2] package/cvs: fix mktime related compile failure
From: Thomas Petazzoni @ 2020-05-29 19:07 UTC (permalink / raw)
To: buildroot
In-Reply-To: <20200529204343.4b3c92bb@gmx.net>
On Fri, 29 May 2020 20:43:43 +0200
Peter Seiderer <ps.report@gmx.net> wrote:
> > With the time_t conversion to 64bit, time_t *is* larger than an
> > unsigned int.
> >
> > So aren't you papering over the problem, and in fact potentially
> > causing some issues in CVS, which seems to assume the time_t fits in
> > 32-bit ?
>
> ....it is only the internal assumption of the local mktime implementation,
> as far as I see all cvs callers of mktime use time_t to store the return
> value....
Hm, ok. But does it make sense to use the AlpineLinux patch instead ?
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
From: Logan Gunthorpe @ 2020-05-29 19:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: kvm, David Airlie, Joonas Lahtinen, dri-devel, Bjorn Andersson,
linux-tegra, Julien Grall, Thierry Reding, Will Deacon,
Marek Szyprowski, Jean-Philippe Brucker, linux-samsung-soc,
Marc Zyngier, Krzysztof Kozlowski, Jonathan Hunter,
linux-rockchip, Andy Gross, linux-arm-kernel, linux-s390,
linux-arm-msm, intel-gfx, Jani Nikula, Alex Williamson,
linux-mediatek, Rodrigo Vivi, Matthias Brugger, Thomas Gleixner,
virtualization, Gerald Schaefer, David Woodhouse, Cornelia Huck,
linux-kernel, Tom Murphy, iommu, Kukjin Kim, Robin Murphy
In-Reply-To: <20200529124523.GA11817@infradead.org>
On 2020-05-29 6:45 a.m., Christoph Hellwig wrote:
> On Thu, May 28, 2020 at 06:00:44PM -0600, Logan Gunthorpe wrote:
>>> This issue is most likely in the i915 driver and is most likely caused by the driver not respecting the return value of the dma_map_ops::map_sg function. You can see the driver ignoring the return value here:
>>> https://github.com/torvalds/linux/blob/7e0165b2f1a912a06e381e91f0f4e495f4ac3736/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c#L51
>>>
>>> Previously this didn’t cause issues because the intel map_sg always returned the same number of elements as the input scatter gather list but with the change to this dma-iommu api this is no longer the case. I wasn’t able to track the bug down to a specific line of code unfortunately.
>
> Mark did a big audit into the map_sg API abuse and initially had
> some i915 patches, but then gave up on them with this comment:
>
> "The biggest TODO is DRM/i915 driver and I don't feel brave enough to fix
> it fully. The driver creatively uses sg_table->orig_nents to store the
> size of the allocate scatterlist and ignores the number of the entries
> returned by dma_map_sg function. In this patchset I only fixed the
> sg_table objects exported by dmabuf related functions. I hope that I
> didn't break anything there."
>
> it would be really nice if the i915 maintainers could help with sorting
> that API abuse out.
>
I agree completely that the API abuse should be sorted out, but I think
that's much larger than just the i915 driver. Pretty much every dma-buf
map_dma_buf implementation I looked at ignores the returned nents of
sg_attrs. This sucks, but I don't think it's the bug Tom ran into. See:
amdgpu_dma_buf_map
armada_gem_prime_map_dma_buf
drm_gem_map_dma_buf
i915_gem_map_dma_buf
tegra_gem_prime_map_dma_buf
So this should probably be addressed by the whole GPU community.
However, as Robin pointed out, there are other ugly tricks like stopping
iterating through the SGL when sg_dma_len() is zero. For example, the
AMD driver appears to use drm_prime_sg_to_page_addr_arrays() which does
this trick and thus likely isn't buggy (otherwise, I'd expect someone to
have complained by now seeing AMD has already switched to IOMMU-DMA.
As I tried to point out in my previous email, i915 does not do this
trick. In fact, it completely ignores sg_dma_len() and is thus
completely broken. See i915_scatterlist.h and the __sgt_iter() function.
So it doesn't sound to me like Mark's fix would address the issue at
all. Per my previous email, I'd guess that it can be fixed simply by
adjusting the __sgt_iter() function to do something more sensible.
(Better yet, if possible, ditch __sgt_iter() and use the common DRM
function that AMD uses).
This would at least allow us to make progress with Tom's IOMMU-DMA patch
set and once that gets in, it will be harder for other drivers to make
the same mistake.
Logan
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply
* Re: [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
From: Logan Gunthorpe @ 2020-05-29 19:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: kvm, David Airlie, Joonas Lahtinen, dri-devel, Bjorn Andersson,
linux-tegra, Julien Grall, Thierry Reding, Will Deacon,
Marek Szyprowski, Jean-Philippe Brucker, linux-samsung-soc,
Marc Zyngier, Krzysztof Kozlowski, Jonathan Hunter,
linux-rockchip, Andy Gross, linux-arm-kernel, linux-s390,
linux-arm-msm, intel-gfx, Jani Nikula, Alex Williamson,
linux-mediatek, Rodrigo Vivi, Matthias Brugger, Thomas Gleixner,
virtualization, Gerald Schaefer, David Woodhouse, Cornelia Huck,
linux-kernel, Tom Murphy, iommu, Kukjin Kim, Robin Murphy
In-Reply-To: <20200529124523.GA11817@infradead.org>
On 2020-05-29 6:45 a.m., Christoph Hellwig wrote:
> On Thu, May 28, 2020 at 06:00:44PM -0600, Logan Gunthorpe wrote:
>>> This issue is most likely in the i915 driver and is most likely caused by the driver not respecting the return value of the dma_map_ops::map_sg function. You can see the driver ignoring the return value here:
>>> https://github.com/torvalds/linux/blob/7e0165b2f1a912a06e381e91f0f4e495f4ac3736/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c#L51
>>>
>>> Previously this didn’t cause issues because the intel map_sg always returned the same number of elements as the input scatter gather list but with the change to this dma-iommu api this is no longer the case. I wasn’t able to track the bug down to a specific line of code unfortunately.
>
> Mark did a big audit into the map_sg API abuse and initially had
> some i915 patches, but then gave up on them with this comment:
>
> "The biggest TODO is DRM/i915 driver and I don't feel brave enough to fix
> it fully. The driver creatively uses sg_table->orig_nents to store the
> size of the allocate scatterlist and ignores the number of the entries
> returned by dma_map_sg function. In this patchset I only fixed the
> sg_table objects exported by dmabuf related functions. I hope that I
> didn't break anything there."
>
> it would be really nice if the i915 maintainers could help with sorting
> that API abuse out.
>
I agree completely that the API abuse should be sorted out, but I think
that's much larger than just the i915 driver. Pretty much every dma-buf
map_dma_buf implementation I looked at ignores the returned nents of
sg_attrs. This sucks, but I don't think it's the bug Tom ran into. See:
amdgpu_dma_buf_map
armada_gem_prime_map_dma_buf
drm_gem_map_dma_buf
i915_gem_map_dma_buf
tegra_gem_prime_map_dma_buf
So this should probably be addressed by the whole GPU community.
However, as Robin pointed out, there are other ugly tricks like stopping
iterating through the SGL when sg_dma_len() is zero. For example, the
AMD driver appears to use drm_prime_sg_to_page_addr_arrays() which does
this trick and thus likely isn't buggy (otherwise, I'd expect someone to
have complained by now seeing AMD has already switched to IOMMU-DMA.
As I tried to point out in my previous email, i915 does not do this
trick. In fact, it completely ignores sg_dma_len() and is thus
completely broken. See i915_scatterlist.h and the __sgt_iter() function.
So it doesn't sound to me like Mark's fix would address the issue at
all. Per my previous email, I'd guess that it can be fixed simply by
adjusting the __sgt_iter() function to do something more sensible.
(Better yet, if possible, ditch __sgt_iter() and use the common DRM
function that AMD uses).
This would at least allow us to make progress with Tom's IOMMU-DMA patch
set and once that gets in, it will be harder for other drivers to make
the same mistake.
Logan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ASoC: SOF: Intel: hda: fix generic hda codec support
From: Mark Brown @ 2020-05-29 19:04 UTC (permalink / raw)
To: alsa-devel, Kai Vehmanen
Cc: Hui Wang, Guennadi Liakhovetski, daniel.baluta,
pierre-louis.bossart, ranjani.sridharan
In-Reply-To: <20200529160358.12134-1-kai.vehmanen@linux.intel.com>
On Fri, 29 May 2020 19:03:58 +0300, Kai Vehmanen wrote:
> Add support for using generic codec driver with SOF. Generic driver
> is used if:
> - snd_sof_intel_hda_common.hda_model="generic" is set, or
> - fallback if no other codec driver is found
>
> The implementation is aligned with snd-hda-intel driver, and fixes audio
> support for systems like Acer Swift 3 SF314-57G, on which this issue was
> originally reported.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: SOF: Intel: hda: fix generic hda codec support
commit: 89d73ccab20a684d8446cea4d8ac6a2608c8d390
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
From: Logan Gunthorpe @ 2020-05-29 19:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Tom Murphy, iommu, kvm, David Airlie, dri-devel, Bjorn Andersson,
Matthias Brugger, Julien Grall, Thierry Reding, Will Deacon,
Jean-Philippe Brucker, linux-samsung-soc, Marc Zyngier,
Krzysztof Kozlowski, Jonathan Hunter, linux-rockchip, Andy Gross,
Gerald Schaefer, linux-s390, linux-arm-msm, intel-gfx,
Alex Williamson, linux-mediatek, Rodrigo Vivi, linux-tegra,
Thomas Gleixner, virtualization, linux-arm-kernel, Robin Murphy,
Cornelia Huck, linux-kernel, Kukjin Kim, David Woodhouse,
Marek Szyprowski, Jani Nikula, Joonas Lahtinen
In-Reply-To: <20200529124523.GA11817@infradead.org>
On 2020-05-29 6:45 a.m., Christoph Hellwig wrote:
> On Thu, May 28, 2020 at 06:00:44PM -0600, Logan Gunthorpe wrote:
>>> This issue is most likely in the i915 driver and is most likely caused by the driver not respecting the return value of the dma_map_ops::map_sg function. You can see the driver ignoring the return value here:
>>> https://github.com/torvalds/linux/blob/7e0165b2f1a912a06e381e91f0f4e495f4ac3736/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c#L51
>>>
>>> Previously this didn’t cause issues because the intel map_sg always returned the same number of elements as the input scatter gather list but with the change to this dma-iommu api this is no longer the case. I wasn’t able to track the bug down to a specific line of code unfortunately.
>
> Mark did a big audit into the map_sg API abuse and initially had
> some i915 patches, but then gave up on them with this comment:
>
> "The biggest TODO is DRM/i915 driver and I don't feel brave enough to fix
> it fully. The driver creatively uses sg_table->orig_nents to store the
> size of the allocate scatterlist and ignores the number of the entries
> returned by dma_map_sg function. In this patchset I only fixed the
> sg_table objects exported by dmabuf related functions. I hope that I
> didn't break anything there."
>
> it would be really nice if the i915 maintainers could help with sorting
> that API abuse out.
>
I agree completely that the API abuse should be sorted out, but I think
that's much larger than just the i915 driver. Pretty much every dma-buf
map_dma_buf implementation I looked at ignores the returned nents of
sg_attrs. This sucks, but I don't think it's the bug Tom ran into. See:
amdgpu_dma_buf_map
armada_gem_prime_map_dma_buf
drm_gem_map_dma_buf
i915_gem_map_dma_buf
tegra_gem_prime_map_dma_buf
So this should probably be addressed by the whole GPU community.
However, as Robin pointed out, there are other ugly tricks like stopping
iterating through the SGL when sg_dma_len() is zero. For example, the
AMD driver appears to use drm_prime_sg_to_page_addr_arrays() which does
this trick and thus likely isn't buggy (otherwise, I'd expect someone to
have complained by now seeing AMD has already switched to IOMMU-DMA.
As I tried to point out in my previous email, i915 does not do this
trick. In fact, it completely ignores sg_dma_len() and is thus
completely broken. See i915_scatterlist.h and the __sgt_iter() function.
So it doesn't sound to me like Mark's fix would address the issue at
all. Per my previous email, I'd guess that it can be fixed simply by
adjusting the __sgt_iter() function to do something more sensible.
(Better yet, if possible, ditch __sgt_iter() and use the common DRM
function that AMD uses).
This would at least allow us to make progress with Tom's IOMMU-DMA patch
set and once that gets in, it will be harder for other drivers to make
the same mistake.
Logan
^ permalink raw reply
* Re: [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
From: Logan Gunthorpe @ 2020-05-29 19:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: kvm, David Airlie, Joonas Lahtinen, dri-devel, Bjorn Andersson,
linux-tegra, Julien Grall, Thierry Reding, Will Deacon,
Jean-Philippe Brucker, linux-samsung-soc, Marc Zyngier,
Krzysztof Kozlowski, Jonathan Hunter, linux-rockchip, Andy Gross,
linux-arm-kernel, linux-s390, linux-arm-msm, intel-gfx,
Jani Nikula, Alex Williamson, linux-mediatek, Rodrigo Vivi,
Matthias Brugger, Thomas Gleixner, virtualization,
Gerald Schaefer, David Woodhouse, Cornelia Huck, linux-kernel,
Tom Murphy, iommu, Kukjin Kim, Robin Murphy
In-Reply-To: <20200529124523.GA11817@infradead.org>
On 2020-05-29 6:45 a.m., Christoph Hellwig wrote:
> On Thu, May 28, 2020 at 06:00:44PM -0600, Logan Gunthorpe wrote:
>>> This issue is most likely in the i915 driver and is most likely caused by the driver not respecting the return value of the dma_map_ops::map_sg function. You can see the driver ignoring the return value here:
>>> https://github.com/torvalds/linux/blob/7e0165b2f1a912a06e381e91f0f4e495f4ac3736/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c#L51
>>>
>>> Previously this didn’t cause issues because the intel map_sg always returned the same number of elements as the input scatter gather list but with the change to this dma-iommu api this is no longer the case. I wasn’t able to track the bug down to a specific line of code unfortunately.
>
> Mark did a big audit into the map_sg API abuse and initially had
> some i915 patches, but then gave up on them with this comment:
>
> "The biggest TODO is DRM/i915 driver and I don't feel brave enough to fix
> it fully. The driver creatively uses sg_table->orig_nents to store the
> size of the allocate scatterlist and ignores the number of the entries
> returned by dma_map_sg function. In this patchset I only fixed the
> sg_table objects exported by dmabuf related functions. I hope that I
> didn't break anything there."
>
> it would be really nice if the i915 maintainers could help with sorting
> that API abuse out.
>
I agree completely that the API abuse should be sorted out, but I think
that's much larger than just the i915 driver. Pretty much every dma-buf
map_dma_buf implementation I looked at ignores the returned nents of
sg_attrs. This sucks, but I don't think it's the bug Tom ran into. See:
amdgpu_dma_buf_map
armada_gem_prime_map_dma_buf
drm_gem_map_dma_buf
i915_gem_map_dma_buf
tegra_gem_prime_map_dma_buf
So this should probably be addressed by the whole GPU community.
However, as Robin pointed out, there are other ugly tricks like stopping
iterating through the SGL when sg_dma_len() is zero. For example, the
AMD driver appears to use drm_prime_sg_to_page_addr_arrays() which does
this trick and thus likely isn't buggy (otherwise, I'd expect someone to
have complained by now seeing AMD has already switched to IOMMU-DMA.
As I tried to point out in my previous email, i915 does not do this
trick. In fact, it completely ignores sg_dma_len() and is thus
completely broken. See i915_scatterlist.h and the __sgt_iter() function.
So it doesn't sound to me like Mark's fix would address the issue at
all. Per my previous email, I'd guess that it can be fixed simply by
adjusting the __sgt_iter() function to do something more sensible.
(Better yet, if possible, ditch __sgt_iter() and use the common DRM
function that AMD uses).
This would at least allow us to make progress with Tom's IOMMU-DMA patch
set and once that gets in, it will be harder for other drivers to make
the same mistake.
Logan
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 0/8] Convert the intel iommu driver to the dma-iommu api
From: Logan Gunthorpe @ 2020-05-29 19:05 UTC (permalink / raw)
To: Christoph Hellwig
Cc: kvm, David Airlie, dri-devel, Bjorn Andersson, linux-tegra,
Julien Grall, Will Deacon, Marek Szyprowski,
Jean-Philippe Brucker, linux-samsung-soc, Marc Zyngier,
Krzysztof Kozlowski, Jonathan Hunter, linux-rockchip, Andy Gross,
linux-arm-kernel, linux-s390, linux-arm-msm, intel-gfx,
linux-mediatek, Matthias Brugger, Thomas Gleixner, virtualization,
Gerald Schaefer, David Woodhouse, Cornelia Huck, linux-kernel,
Tom Murphy, iommu, Kukjin Kim, Robin Murphy
In-Reply-To: <20200529124523.GA11817@infradead.org>
On 2020-05-29 6:45 a.m., Christoph Hellwig wrote:
> On Thu, May 28, 2020 at 06:00:44PM -0600, Logan Gunthorpe wrote:
>>> This issue is most likely in the i915 driver and is most likely caused by the driver not respecting the return value of the dma_map_ops::map_sg function. You can see the driver ignoring the return value here:
>>> https://github.com/torvalds/linux/blob/7e0165b2f1a912a06e381e91f0f4e495f4ac3736/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c#L51
>>>
>>> Previously this didn’t cause issues because the intel map_sg always returned the same number of elements as the input scatter gather list but with the change to this dma-iommu api this is no longer the case. I wasn’t able to track the bug down to a specific line of code unfortunately.
>
> Mark did a big audit into the map_sg API abuse and initially had
> some i915 patches, but then gave up on them with this comment:
>
> "The biggest TODO is DRM/i915 driver and I don't feel brave enough to fix
> it fully. The driver creatively uses sg_table->orig_nents to store the
> size of the allocate scatterlist and ignores the number of the entries
> returned by dma_map_sg function. In this patchset I only fixed the
> sg_table objects exported by dmabuf related functions. I hope that I
> didn't break anything there."
>
> it would be really nice if the i915 maintainers could help with sorting
> that API abuse out.
>
I agree completely that the API abuse should be sorted out, but I think
that's much larger than just the i915 driver. Pretty much every dma-buf
map_dma_buf implementation I looked at ignores the returned nents of
sg_attrs. This sucks, but I don't think it's the bug Tom ran into. See:
amdgpu_dma_buf_map
armada_gem_prime_map_dma_buf
drm_gem_map_dma_buf
i915_gem_map_dma_buf
tegra_gem_prime_map_dma_buf
So this should probably be addressed by the whole GPU community.
However, as Robin pointed out, there are other ugly tricks like stopping
iterating through the SGL when sg_dma_len() is zero. For example, the
AMD driver appears to use drm_prime_sg_to_page_addr_arrays() which does
this trick and thus likely isn't buggy (otherwise, I'd expect someone to
have complained by now seeing AMD has already switched to IOMMU-DMA.
As I tried to point out in my previous email, i915 does not do this
trick. In fact, it completely ignores sg_dma_len() and is thus
completely broken. See i915_scatterlist.h and the __sgt_iter() function.
So it doesn't sound to me like Mark's fix would address the issue at
all. Per my previous email, I'd guess that it can be fixed simply by
adjusting the __sgt_iter() function to do something more sensible.
(Better yet, if possible, ditch __sgt_iter() and use the common DRM
function that AMD uses).
This would at least allow us to make progress with Tom's IOMMU-DMA patch
set and once that gets in, it will be harder for other drivers to make
the same mistake.
Logan
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* [xen-unstable-smoke test] 150498: regressions - trouble: blocked/fail
From: osstest service owner @ 2020-05-29 19:05 UTC (permalink / raw)
To: xen-devel, osstest-admin
flight 150498 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/150498/
Regressions :-(
Tests which did not succeed and are blocking,
including tests which could not be run:
build-arm64-xsm 6 xen-build fail REGR. vs. 150438
build-amd64 6 xen-build fail REGR. vs. 150438
build-armhf 6 xen-build fail REGR. vs. 150438
Tests which did not succeed, but are not blocking:
test-amd64-amd64-libvirt 1 build-check(1) blocked n/a
test-armhf-armhf-xl 1 build-check(1) blocked n/a
test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a
test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a
build-amd64-libvirt 1 build-check(1) blocked n/a
version targeted for testing:
xen 8e2aa76dc1670e82eaa15683353853bc66bf54fc
baseline version:
xen 1497e78068421d83956f8e82fb6e1bf1fc3b1199
Last test of basis 150438 2020-05-28 14:01:19 Z 1 days
Failing since 150465 2020-05-29 09:02:14 Z 0 days 7 attempts
Testing same since 150498 2020-05-29 18:01:30 Z 0 days 1 attempts
------------------------------------------------------------
People who touched revisions under test:
Andrew Cooper <andrew.cooper3@citrix.com>
Andrew Cooper <andrew.cooper@citrix.com>
Dario Faggioli <dfaggioli@suse.com>
Ian Jackson <ian.jackson@eu.citrix.com>
Jan Beulich <jbeulich@suse.com>
Juergen Gross <jgross@suse.com>
Julien Grall <jgrall@amazon.com>
Roger Pau Monné <roger.pau@citrix.com>
Tamas K Lengyel <tamas@tklengyel.com>
Wei Liu <wl@xen.org>
jobs:
build-arm64-xsm fail
build-amd64 fail
build-armhf fail
build-amd64-libvirt blocked
test-armhf-armhf-xl blocked
test-arm64-arm64-xl-xsm blocked
test-amd64-amd64-xl-qemuu-debianhvm-amd64 blocked
test-amd64-amd64-libvirt blocked
------------------------------------------------------------
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images
Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs
Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master
Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary
Not pushing.
(No revision log; it would be 773 lines long.)
^ permalink raw reply
* Re: Bug: toolstack allows too low values to be set for shadow_memory
From: Hans van Kranenburg @ 2020-05-29 19:05 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel@lists.xenproject.org
In-Reply-To: <304a7a55-20a6-1dfe-1f3a-dabe90d28f40@suse.com>
Hi,
On 5/26/20 9:41 AM, Jan Beulich wrote:
> On 25.05.2020 17:51, Hans van Kranenburg wrote:
>> This bug report is a follow-up to the thread "Domu windows 2012 crash"
>> on the xen-users list. In there we found out that it is possible to set
>> a value for shadow_memory that is lower than a safe minimum value.
>>
>> This became apparent after XSA-304, which caused using more of this type
>> of memory. Having a hardcoded line like shadow_memory = 8 results in
>> random crashes of the guest,
>
> I don't think it is the tool stack's responsibility to override
> admin requested values, or at least not as far a affecting guest
> stability goes;
This is not primarily a technical issue, or about if software works
correct in a mathematical proven way.
It's a usability issue, so it's about what levels of unknowingly
unloading guns into feet is deemed desirable.
And, if that's happening, how difficult it should be for a user to
actually find out what's wrong.
> host stability of course needs to be guaranteed,
> but that's then still the hypervisor's job, not the tool stack's.
>
> Compare this to e.g. setting too small a memory= for a guest to
> be able to boot at all, or setting maxmem > memory for a guest
> without balloon driver.
>
> Furthermore - what would the suggestion be as to a "safe minimum
> value"? Assuming _all_ large pages may potentially get shattered
> is surely a waste of memory, unless the admin really knows
> guests are going to behave that way. (In your report you also
> didn't mention what memory= values the issue was observed with.
> Obviously larger memory= also require bumping shadow_memory= at
> least from some point onwards.)
Thanks,
Hans
^ permalink raw reply
* Re: [PATCH v2 11/12] dt-bindings: iio: imu: Add inv_icm42600 documentation
From: Rob Herring @ 2020-05-29 19:05 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol
Cc: jic23, mchehab+huawei, davem, gregkh, linux-iio, devicetree,
linux-kernel
In-Reply-To: <20200527185711.21331-12-jmaneyrol@invensense.com>
On Wed, May 27, 2020 at 08:57:10PM +0200, Jean-Baptiste Maneyrol wrote:
> Document the ICM-426xxx devices devicetree bindings.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> ---
> .../bindings/iio/imu/invensense,icm42600.yaml | 86 +++++++++++++++++++
> 1 file changed, 86 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> new file mode 100644
> index 000000000000..c5b046e0ce36
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/imu/invensense,icm42600.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: InvenSense ICM-426xx Inertial Measurement Unit
> +
> +maintainers:
> + - Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
> +
> +description: |
> + 6-axis MotionTracking device that combines a 3-axis gyroscope and a 3-axis accelerometer.
> +
> + It has a configurable host interface that supports I3C, I2C and SPI serial communication, features a 2kB FIFO and
> + 2 programmable interrupts with ultra-low-power wake-on-motion support to minimize system power consumption.
> +
> + Other industry-leading features include InvenSense on-chip APEX Motion Processing engine for gesture recognition,
> + activity classification, and pedometer, along with programmable digital filters, and an embedded temperature sensor.
Wrap lines at <80 chars.
> +
> + https://invensense.tdk.com/wp-content/uploads/2020/03/DS-000292-ICM-42605-v1.4.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - invensense,icm42600
> + - invensense,icm42602
> + - invensense,icm42605
> + - invensense,icm42622
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + drive-open-drain:
> + type: boolean
> +
> + vdd-supply:
> + description: Regulator that provides power to the sensor
> +
> + vddio-supply:
> + description: Regulator that provides power to the bus
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + i2c0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + icm42605@68 {
> + compatible = "invensense,icm42605";
> + reg = <0x68>;
> + interrupt-parent = <&gpio2>;
> + interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> + vdd-supply = <&vdd>;
> + vddio-supply = <&vddio>;
> + };
> + };
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + spi0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + icm42602@0 {
> + compatible = "invensense,icm42602";
> + reg = <0>;
> + spi-max-frequency = <24000000>;
> + spi-cpha;
> + spi-cpol;
> + interrupt-parent = <&gpio1>;
> + interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> + vdd-supply = <&vdd>;
> + vddio-supply = <&vddio>;
> + };
> + };
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH 1/6] x86/entry: Introduce local_db_{save,restore}()
From: Peter Zijlstra @ 2020-05-29 19:02 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andrew Cooper, tglx, linux-kernel, x86, Lai Jiangshan,
sean.j.christopherson, daniel.thompson
In-Reply-To: <AC94E789-9EC7-4156-955C-841FF7114176@amacapital.net>
On Fri, May 29, 2020 at 10:28:33AM -0700, Andy Lutomirski wrote:
> > +static __always_inline unsigned long local_db_save(void)
> > +{
> > + unsigned long dr7;
> > +
> > + get_debugreg(&dr7, 7);
> > + dr7 ^= 0x400;
>
> Why xor? This seems extra confusing.
I'll do the normal mask thing ..
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
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.