Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v2] arm64: fault: Don't leak data in ESR context for user fault on kernel VA
From: Will Deacon @ 2018-05-22 13:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180419154833.27727-1-peter.maydell@linaro.org>

Hi Peter,

Sorry for the delay in getting to this! Comments inline.

On Thu, Apr 19, 2018 at 04:48:33PM +0100, Peter Maydell wrote:
> If userspace faults on a kernel address, handing them the raw ESR
> value on the sigframe as part of the delivered signal can leak data
> useful to attackers who are using information about the underlying hardware
> fault type (e.g. translation vs permission) as a mechanism to defeat KASLR.
> 
> However there are also legitimate uses for the information provided
> in the ESR -- notably the GCC and LLVM sanitizers use this to report
> whether wild pointer accesses by the application are reads or writes
> (since a wild write is a more serious bug than a wild read), so we
> don't want to drop the ESR information entirely.
> 
> For faulting addresses in the kernel, sanitize the ESR. We choose
> to present userspace with the illusion that there is nothing mapped
> in the kernel's part of the address space at all, by reporting all
> faults as level 0 translation faults.
> 
> These fields are safe to pass through to userspace as they depend
> only on the instruction that userspace used to provoke the fault:
>  EC IL (always)
>  ISV CM WNR (for all data aborts)
>  SAS SSE SRT SF AR (for data aborts when ISV is 1)
> All the other fields in ESR except DFSC are architecturally RES0
> for an L0 translation fault, so can be zeroed out without confusing
> userspace.
> 
> The illusion is not entirely perfect, as there is a tiny wrinkle
> where we will report an alignment fault that was not due to the memory
> type (for instance a LDREX to an unaligned address) as a translation
> fault, whereas if you do this on real unmapped memory the alignment
> fault takes precedence. This is not likely to trip anybody up in
> practice, as the only users we know of for the ESR information who
> care about the behaviour for kernel addresses only really want to
> know about the WnR bit.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This RFC patch is an alternative proposal to Will's patch
> https://patchwork.kernel.org/patch/10258781/
> which simply removed the ESR record entirely for kernel addresses.
> 
> Changes v1->v2:
>  * rebased on master
>  * commit message tweak
>  * DABT_CUR and IABT_CUR moved to "can't happen" default case
>  * explicitly clear the bits which are RES0 if ISV == 0
>  * comment text tweaks
> ---
>  arch/arm64/mm/fault.c | 55 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 4165485e8b6e..8fa78fa01a4a 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -293,6 +293,61 @@ static void __do_kernel_fault(unsigned long addr, unsigned int esr,
>  static void __do_user_fault(struct siginfo *info, unsigned int esr)
>  {
>  	current->thread.fault_address = (unsigned long)info->si_addr;
> +
> +	/*
> +	 * If the faulting address is in the kernel, we must sanitize the ESR.
> +	 * From userspace's point of view, kernel-only mappings don't exist
> +	 * at all, so we report them as level 0 translation faults.
> +	 * (This is not quite the way that "no mapping there at all" behaves:
> +	 * an alignment fault not caused by the memory type would take
> +	 * precedence over translation fault for a real access to empty
> +	 * space. Unfortunately we can't easily distinguish "alignment fault
> +	 * not caused by memory type" from "alignment fault caused by memory
> +	 * type", so we ignore this wrinkle and just return the translation
> +	 * fault.)
> +	 */
> +	if (current->thread.fault_address >= TASK_SIZE) {
> +		switch (ESR_ELx_EC(esr)) {
> +		case ESR_ELx_EC_DABT_LOW:
> +			/*
> +			 * These bits provide only information about the
> +			 * faulting instruction, which userspace knows already.
> +			 * We explicitly clear bits which are architecturally
> +			 * RES0 in case they are given meanings in future.
> +			 */
> +			if (esr & ESR_ELx_ISV)
> +				esr &= ESR_ELx_EC_MASK | ESR_ELx_IL |
> +					ESR_ELx_ISV | ESR_ELx_SAS |
> +					ESR_ELx_SSE | ESR_ELx_SRT_MASK |
> +					ESR_ELx_SF | ESR_ELx_AR | ESR_ELx_CM |
> +					ESR_ELx_WNR;

Reading through the ARM ARM, it seems to say that ISV is always 0 for
faults reported in ESR_EL1, which implies we can drop ISV, SAS, SSE, SRT,
SF and AR from this list and actually drop the conditional altogether.

Will

^ permalink raw reply

* [PATCH 12/14] OMAP: CLK: CLKSRC: Add suspend resume hooks
From: Tony Lindgren @ 2018-05-22 13:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <038cc2cd-ff72-c9c9-257b-1c1cc09a66a8@ti.com>

* Keerthy <j-keerthy@ti.com> [180522 07:59]:
> On Thursday 12 April 2018 07:57 PM, Tony Lindgren wrote:
> > * Keerthy <j-keerthy@ti.com> [180412 03:56]:
> >> Add the save and restore for clksrc as part of suspend and resume
> >> so that it saves the counter value and restores. This is needed in
> >> modes like rtc+ddr in self-refresh not doing this stalls the time.
> > 
> > I suspect this too should really happen with cpu_pm.
> 
> I believe going by the previous set of patches this fits better with
> suspend/resume?

Yes if you don't need it for cpuidle and other SoC variants
don't need it for cpuidle.

> > Probably no need to look up the hwmod every time? Especially if am437x
> > will start supporting deeper idle modes during runtime.
> 
> Like clockevent i will store the hwmod pointer for clocksource as well.

OK

Regards,

Tony

^ permalink raw reply

* [PATCH RFC V2 2/6] hwmon: Add support for RPi voltage sensor
From: Guenter Roeck @ 2018-05-22 13:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526988112-4021-3-git-send-email-stefan.wahren@i2se.com>

On 05/22/2018 04:21 AM, Stefan Wahren wrote:
> Currently there is no easy way to detect undervoltage conditions on a
> remote Raspberry Pi. This hwmon driver retrieves the state of the
> undervoltage sensor via mailbox interface. The handling based on
> Noralf's modifications to the downstream firmware driver. In case of
> an undervoltage condition only an entry is written to the kernel log.
> 
> CC: "Noralf Tr?nnes" <noralf@tronnes.org>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>   Documentation/hwmon/raspberrypi-hwmon |  22 +++++
>   drivers/hwmon/Kconfig                 |  10 ++
>   drivers/hwmon/Makefile                |   1 +
>   drivers/hwmon/raspberrypi-hwmon.c     | 168 ++++++++++++++++++++++++++++++++++
>   4 files changed, 201 insertions(+)
>   create mode 100644 Documentation/hwmon/raspberrypi-hwmon
>   create mode 100644 drivers/hwmon/raspberrypi-hwmon.c
> 
> diff --git a/Documentation/hwmon/raspberrypi-hwmon b/Documentation/hwmon/raspberrypi-hwmon
> new file mode 100644
> index 0000000..3c92e2c
> --- /dev/null
> +++ b/Documentation/hwmon/raspberrypi-hwmon
> @@ -0,0 +1,22 @@
> +Kernel driver raspberrypi-hwmon
> +===============================
> +
> +Supported boards:
> +  * Raspberry Pi A+ (via GPIO on SoC)
> +  * Raspberry Pi B+ (via GPIO on SoC)
> +  * Raspberry Pi 2 B (via GPIO on SoC)
> +  * Raspberry Pi 3 B (via GPIO on port expander)
> +  * Raspberry Pi 3 B+ (via PMIC)
> +
> +Author: Stefan Wahren <stefan.wahren@i2se.com>
> +
> +Description
> +-----------
> +
> +This driver periodically polls a mailbox property of the VC4 firmware to detect
> +undervoltage conditions.
> +
> +Sysfs entries
> +-------------
> +
> +in0_lcrit_alarm		Undervoltage alarm
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 768aed5..9a5bdb0 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1298,6 +1298,16 @@ config SENSORS_PWM_FAN
>   	  This driver can also be built as a module.  If so, the module
>   	  will be called pwm-fan.
>   
> +config SENSORS_RASPBERRYPI_HWMON
> +	tristate "Raspberry Pi voltage monitor"
> +	depends on RASPBERRYPI_FIRMWARE || COMPILE_TEST
> +	help
> +	  If you say yes here you get support for voltage sensor on the
> +	  Raspberry Pi.
> +
> +	  This driver can also be built as a module. If so, the module
> +	  will be called raspberrypi-hwmon.
> +
>   config SENSORS_SHT15
>   	tristate "Sensiron humidity and temperature sensors. SHT15 and compat."
>   	depends on GPIOLIB || COMPILE_TEST
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index e7d52a3..a929770 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -141,6 +141,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
>   obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
>   obj-$(CONFIG_SENSORS_POWR1220)  += powr1220.o
>   obj-$(CONFIG_SENSORS_PWM_FAN)	+= pwm-fan.o
> +obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON)	+= raspberrypi-hwmon.o
>   obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
>   obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
>   obj-$(CONFIG_SENSORS_SCH5627)	+= sch5627.o
> diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
> new file mode 100644
> index 0000000..6233e84
> --- /dev/null
> +++ b/drivers/hwmon/raspberrypi-hwmon.c
> @@ -0,0 +1,168 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Raspberry Pi voltage sensor driver
> + *
> + * Based on firmware/raspberrypi.c by Noralf Tr?nnes
> + *
> + * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
> + */
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +#include <soc/bcm2835/raspberrypi-firmware.h>
> +
> +#define UNDERVOLTAGE_STICKY_BIT	BIT(16)
> +
> +struct rpi_hwmon_data {
> +	struct device *hwmon_dev;
> +	struct rpi_firmware *fw;
> +	u32 last_throttled;
> +	struct delayed_work get_values_poll_work;
> +};
> +
> +static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
> +{
> +	u32 new_uv, old_uv, value;
> +	int ret;
> +
> +	/* Request firmware to clear sticky bits */
> +	value = 0xffff;
> +
> +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
> +				    &value, sizeof(value));
> +	if (ret) {
> +		dev_err_once(data->hwmon_dev, "Failed to get throttled (%d)\n",
> +			     ret);
> +		return;
> +	}
> +
> +	new_uv = value & UNDERVOLTAGE_STICKY_BIT;
> +	old_uv = data->last_throttled & UNDERVOLTAGE_STICKY_BIT;
> +	data->last_throttled = value;
> +
> +	if (new_uv == old_uv)
> +		return;
> +
> +	if (new_uv)
> +		dev_crit(data->hwmon_dev, "Undervoltage detected!\n");
> +	else
> +		dev_info(data->hwmon_dev, "Voltage normalised\n");
> +
> +	sysfs_notify(&data->hwmon_dev->kobj, NULL, "in0_lcrit_alarm");
> +}
> +
> +static void get_values_poll(struct work_struct *work)
> +{
> +	struct rpi_hwmon_data *data;
> +
> +	data = container_of(work, struct rpi_hwmon_data,
> +			    get_values_poll_work.work);
> +
> +	rpi_firmware_get_throttled(data);
> +
> +	/*
> +	 * We can't run faster than the sticky shift (100ms) since we get
> +	 * flipping in the sticky bits that are cleared.
> +	 */
> +	schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
> +}
> +
> +static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
> +		    u32 attr, int channel, long *val)
> +{
> +	struct rpi_hwmon_data *data = dev_get_drvdata(dev);
> +
> +	*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
> +	return 0;
> +}
> +
> +static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
> +			      u32 attr, int channel)
> +{
> +	return 0444;
> +}
> +
> +static const u32 rpi_in_config[] = {
> +	HWMON_I_LCRIT_ALARM,
> +	0
> +};
> +
> +static const struct hwmon_channel_info rpi_in = {
> +	.type = hwmon_in,
> +	.config = rpi_in_config,
> +};
> +
> +static const struct hwmon_channel_info *rpi_info[] = {
> +	&rpi_in,
> +	NULL
> +};
> +
> +static const struct hwmon_ops rpi_hwmon_ops = {
> +	.is_visible = rpi_is_visible,
> +	.read = rpi_read,
> +};
> +
> +static const struct hwmon_chip_info rpi_chip_info = {
> +	.ops = &rpi_hwmon_ops,
> +	.info = rpi_info,
> +};
> +
> +static int rpi_hwmon_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rpi_hwmon_data *data;
> +	int ret;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->fw = platform_get_drvdata(to_platform_device(dev->parent));
> +	if (!data->fw)
> +		return -EPROBE_DEFER;
> +

I am a bit at loss here (and sorry I didn't bring this up before).
How would this ever be possible, given that the driver is registered
from the firmware driver ?

> +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
> +				    &data->last_throttled,
> +				    sizeof(data->last_throttled));
> +	if (ret)
> +		return -ENODEV;
> +
> +	data->hwmon_dev = devm_hwmon_device_register_with_info(dev, "rpi_volt",
> +							       data,
> +							       &rpi_chip_info,
> +							       NULL);
> +
> +	INIT_DELAYED_WORK(&data->get_values_poll_work, get_values_poll);
> +	platform_set_drvdata(pdev, data);
> +
> +	if (!PTR_ERR_OR_ZERO(data->hwmon_dev))
> +		schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
> +
> +	return PTR_ERR_OR_ZERO(data->hwmon_dev);
> +}
> +
> +static int rpi_hwmon_remove(struct platform_device *pdev)
> +{
> +	struct rpi_hwmon_data *data = platform_get_drvdata(pdev);
> +
> +	cancel_delayed_work_sync(&data->get_values_poll_work);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver rpi_hwmon_driver = {
> +	.probe = rpi_hwmon_probe,
> +	.remove = rpi_hwmon_remove,
> +	.driver = {
> +		.name = "raspberrypi-hwmon",
> +	},
> +};
> +module_platform_driver(rpi_hwmon_driver);
> +
> +MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
> +MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
> +MODULE_LICENSE("GPL v2");
> 

^ permalink raw reply

* [RFC PATCH v2] arm64: fault: Don't leak data in ESR context for user fault on kernel VA
From: Peter Maydell @ 2018-05-22 13:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522133826.GE26955@arm.com>

On 22 May 2018 at 14:38, Will Deacon <will.deacon@arm.com> wrote:
> Hi Peter,
>
> Sorry for the delay in getting to this! Comments inline.
>
> On Thu, Apr 19, 2018 at 04:48:33PM +0100, Peter Maydell wrote:

>> +                     /*
>> +                      * These bits provide only information about the
>> +                      * faulting instruction, which userspace knows already.
>> +                      * We explicitly clear bits which are architecturally
>> +                      * RES0 in case they are given meanings in future.
>> +                      */
>> +                     if (esr & ESR_ELx_ISV)
>> +                             esr &= ESR_ELx_EC_MASK | ESR_ELx_IL |
>> +                                     ESR_ELx_ISV | ESR_ELx_SAS |
>> +                                     ESR_ELx_SSE | ESR_ELx_SRT_MASK |
>> +                                     ESR_ELx_SF | ESR_ELx_AR | ESR_ELx_CM |
>> +                                     ESR_ELx_WNR;
>
> Reading through the ARM ARM, it seems to say that ISV is always 0 for
> faults reported in ESR_EL1, which implies we can drop ISV, SAS, SSE, SRT,
> SF and AR from this list and actually drop the conditional altogether.

Mmm, I guess so, if we're guaranteed to only be working with ESRs
taken to EL1 (or we want to present userspace with an ESR that
looks like that regardless of what EL we took it to). I'll respin
without the conditional.

thanks
-- PMM

^ permalink raw reply

* [PATCH v8 04/15] clk: qcom: Add CPU clock driver for msm8996
From: kbuild test robot @ 2018-05-22 13:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526555955-29960-5-git-send-email-ilialin@codeaurora.org>

Hi Ilia,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.17-rc6]
[cannot apply to clk/clk-next next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ilia-Lin/CPU-scaling-support-for-msm8996/20180520-132401
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> drivers/soc/qcom/kryo-l2-accessors.c:7:10: fatal error: asm/sysreg.h: No such file or directory
    #include <asm/sysreg.h>
             ^~~~~~~~~~~~~~
   compilation terminated.

vim +7 drivers/soc/qcom/kryo-l2-accessors.c

f4e14120 Ilia Lin 2018-05-17 @7  #include <asm/sysreg.h>
f4e14120 Ilia Lin 2018-05-17  8  #include <soc/qcom/kryo-l2-accessors.h>
f4e14120 Ilia Lin 2018-05-17  9  

:::::: The code at line 7 was first introduced by commit
:::::: f4e14120d663ce6e4670516a66bc0158dc692c45 soc: qcom: Separate kryo l2 accessors from PMU driver

:::::: TO: Ilia Lin <ilialin@codeaurora.org>
:::::: CC: 0day robot <lkp@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 64542 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/d0336abb/attachment-0001.gz>

^ permalink raw reply

* [PATCH RFC V2 2/6] hwmon: Add support for RPi voltage sensor
From: Stefan Wahren @ 2018-05-22 13:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <90a768aa-ee8c-1050-cf15-60637069dbdb@roeck-us.net>

Hi Guenter,

> Guenter Roeck <linux@roeck-us.net> hat am 22. Mai 2018 um 15:41 geschrieben:
> 
> 
> On 05/22/2018 04:21 AM, Stefan Wahren wrote:
> > Currently there is no easy way to detect undervoltage conditions on a
> > remote Raspberry Pi. This hwmon driver retrieves the state of the
> > undervoltage sensor via mailbox interface. The handling based on
> > Noralf's modifications to the downstream firmware driver. In case of
> > an undervoltage condition only an entry is written to the kernel log.
> > 
> > CC: "Noralf Tr?nnes" <noralf@tronnes.org>
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > ---
> >   Documentation/hwmon/raspberrypi-hwmon |  22 +++++
> >   drivers/hwmon/Kconfig                 |  10 ++
> >   drivers/hwmon/Makefile                |   1 +
> >   drivers/hwmon/raspberrypi-hwmon.c     | 168 ++++++++++++++++++++++++++++++++++
> >   4 files changed, 201 insertions(+)
> >   create mode 100644 Documentation/hwmon/raspberrypi-hwmon
> >   create mode 100644 drivers/hwmon/raspberrypi-hwmon.c
> > 
> > diff --git a/Documentation/hwmon/raspberrypi-hwmon b/Documentation/hwmon/raspberrypi-hwmon
> > new file mode 100644
> > index 0000000..3c92e2c
> > --- /dev/null
> > +++ b/Documentation/hwmon/raspberrypi-hwmon
> > @@ -0,0 +1,22 @@
> > +Kernel driver raspberrypi-hwmon
> > +===============================
> > +
> > +Supported boards:
> > +  * Raspberry Pi A+ (via GPIO on SoC)
> > +  * Raspberry Pi B+ (via GPIO on SoC)
> > +  * Raspberry Pi 2 B (via GPIO on SoC)
> > +  * Raspberry Pi 3 B (via GPIO on port expander)
> > +  * Raspberry Pi 3 B+ (via PMIC)
> > +
> > +Author: Stefan Wahren <stefan.wahren@i2se.com>
> > +
> > +Description
> > +-----------
> > +
> > +This driver periodically polls a mailbox property of the VC4 firmware to detect
> > +undervoltage conditions.
> > +
> > +Sysfs entries
> > +-------------
> > +
> > +in0_lcrit_alarm		Undervoltage alarm
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 768aed5..9a5bdb0 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -1298,6 +1298,16 @@ config SENSORS_PWM_FAN
> >   	  This driver can also be built as a module.  If so, the module
> >   	  will be called pwm-fan.
> >   
> > +config SENSORS_RASPBERRYPI_HWMON
> > +	tristate "Raspberry Pi voltage monitor"
> > +	depends on RASPBERRYPI_FIRMWARE || COMPILE_TEST
> > +	help
> > +	  If you say yes here you get support for voltage sensor on the
> > +	  Raspberry Pi.
> > +
> > +	  This driver can also be built as a module. If so, the module
> > +	  will be called raspberrypi-hwmon.
> > +
> >   config SENSORS_SHT15
> >   	tristate "Sensiron humidity and temperature sensors. SHT15 and compat."
> >   	depends on GPIOLIB || COMPILE_TEST
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index e7d52a3..a929770 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -141,6 +141,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
> >   obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
> >   obj-$(CONFIG_SENSORS_POWR1220)  += powr1220.o
> >   obj-$(CONFIG_SENSORS_PWM_FAN)	+= pwm-fan.o
> > +obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON)	+= raspberrypi-hwmon.o
> >   obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
> >   obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
> >   obj-$(CONFIG_SENSORS_SCH5627)	+= sch5627.o
> > diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
> > new file mode 100644
> > index 0000000..6233e84
> > --- /dev/null
> > +++ b/drivers/hwmon/raspberrypi-hwmon.c
> > @@ -0,0 +1,168 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Raspberry Pi voltage sensor driver
> > + *
> > + * Based on firmware/raspberrypi.c by Noralf Tr?nnes
> > + *
> > + * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
> > + */
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +#include <linux/workqueue.h>
> > +#include <soc/bcm2835/raspberrypi-firmware.h>
> > +
> > +#define UNDERVOLTAGE_STICKY_BIT	BIT(16)
> > +
> > +struct rpi_hwmon_data {
> > +	struct device *hwmon_dev;
> > +	struct rpi_firmware *fw;
> > +	u32 last_throttled;
> > +	struct delayed_work get_values_poll_work;
> > +};
> > +
> > +static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
> > +{
> > +	u32 new_uv, old_uv, value;
> > +	int ret;
> > +
> > +	/* Request firmware to clear sticky bits */
> > +	value = 0xffff;
> > +
> > +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
> > +				    &value, sizeof(value));
> > +	if (ret) {
> > +		dev_err_once(data->hwmon_dev, "Failed to get throttled (%d)\n",
> > +			     ret);
> > +		return;
> > +	}
> > +
> > +	new_uv = value & UNDERVOLTAGE_STICKY_BIT;
> > +	old_uv = data->last_throttled & UNDERVOLTAGE_STICKY_BIT;
> > +	data->last_throttled = value;
> > +
> > +	if (new_uv == old_uv)
> > +		return;
> > +
> > +	if (new_uv)
> > +		dev_crit(data->hwmon_dev, "Undervoltage detected!\n");
> > +	else
> > +		dev_info(data->hwmon_dev, "Voltage normalised\n");
> > +
> > +	sysfs_notify(&data->hwmon_dev->kobj, NULL, "in0_lcrit_alarm");
> > +}
> > +
> > +static void get_values_poll(struct work_struct *work)
> > +{
> > +	struct rpi_hwmon_data *data;
> > +
> > +	data = container_of(work, struct rpi_hwmon_data,
> > +			    get_values_poll_work.work);
> > +
> > +	rpi_firmware_get_throttled(data);
> > +
> > +	/*
> > +	 * We can't run faster than the sticky shift (100ms) since we get
> > +	 * flipping in the sticky bits that are cleared.
> > +	 */
> > +	schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
> > +}
> > +
> > +static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
> > +		    u32 attr, int channel, long *val)
> > +{
> > +	struct rpi_hwmon_data *data = dev_get_drvdata(dev);
> > +
> > +	*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
> > +	return 0;
> > +}
> > +
> > +static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
> > +			      u32 attr, int channel)
> > +{
> > +	return 0444;
> > +}
> > +
> > +static const u32 rpi_in_config[] = {
> > +	HWMON_I_LCRIT_ALARM,
> > +	0
> > +};
> > +
> > +static const struct hwmon_channel_info rpi_in = {
> > +	.type = hwmon_in,
> > +	.config = rpi_in_config,
> > +};
> > +
> > +static const struct hwmon_channel_info *rpi_info[] = {
> > +	&rpi_in,
> > +	NULL
> > +};
> > +
> > +static const struct hwmon_ops rpi_hwmon_ops = {
> > +	.is_visible = rpi_is_visible,
> > +	.read = rpi_read,
> > +};
> > +
> > +static const struct hwmon_chip_info rpi_chip_info = {
> > +	.ops = &rpi_hwmon_ops,
> > +	.info = rpi_info,
> > +};
> > +
> > +static int rpi_hwmon_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct rpi_hwmon_data *data;
> > +	int ret;
> > +
> > +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > +	if (!data)
> > +		return -ENOMEM;
> > +
> > +	data->fw = platform_get_drvdata(to_platform_device(dev->parent));
> > +	if (!data->fw)
> > +		return -EPROBE_DEFER;
> > +
> 
> I am a bit at loss here (and sorry I didn't bring this up before).
> How would this ever be possible, given that the driver is registered
> from the firmware driver ?

Do you refer to the (wrong) return code, the assumption that the parent must be a platform driver or a possible race?

> 
> > +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
> > +				    &data->last_throttled,
> > +				    sizeof(data->last_throttled));
> > +	if (ret)
> > +		return -ENODEV;
> > +
> > +	data->hwmon_dev = devm_hwmon_device_register_with_info(dev, "rpi_volt",
> > +							       data,
> > +							       &rpi_chip_info,
> > +							       NULL);
> > +
> > +	INIT_DELAYED_WORK(&data->get_values_poll_work, get_values_poll);
> > +	platform_set_drvdata(pdev, data);
> > +
> > +	if (!PTR_ERR_OR_ZERO(data->hwmon_dev))
> > +		schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
> > +
> > +	return PTR_ERR_OR_ZERO(data->hwmon_dev);
> > +}
> > +
> > +static int rpi_hwmon_remove(struct platform_device *pdev)
> > +{
> > +	struct rpi_hwmon_data *data = platform_get_drvdata(pdev);
> > +
> > +	cancel_delayed_work_sync(&data->get_values_poll_work);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver rpi_hwmon_driver = {
> > +	.probe = rpi_hwmon_probe,
> > +	.remove = rpi_hwmon_remove,
> > +	.driver = {
> > +		.name = "raspberrypi-hwmon",
> > +	},
> > +};
> > +module_platform_driver(rpi_hwmon_driver);
> > +
> > +MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
> > +MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
> > +MODULE_LICENSE("GPL v2");
> > 
>

^ permalink raw reply

* [PATCH 13/33] clk: rockchip: use match_string() helper
From: Heiko Stuebner @ 2018-05-22 13:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526903890-35761-14-git-send-email-xieyisheng1@huawei.com>

Am Montag, 21. Mai 2018, 13:57:50 CEST schrieb Yisheng Xie:
> match_string() returns the index of an array for a matching string,
> which can be used intead of open coded variant.
> 
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: linux-clk at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-rockchip at lists.infradead.org
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>

> @@ -312,6 +304,8 @@ static struct clk *rockchip_clk_register_frac_branch(
>  
>  		/* notifier on the fraction divider to catch rate changes */
>  		if (frac->mux_frac_idx >= 0) {
> +			pr_debug("%s: find fractional parent in mux at pos %d\n",
> +				 __func__, frac->mux_frac_idx);

applied to my Rockchip clk branch for 4.18 after changing the "find" above
to "found" again ;-) .


Thanks for the nice cleanup
Heiko

^ permalink raw reply

* [reset-control] How to initialize hardware state with the shared reset line?
From: Philipp Zabel @ 2018-05-22 13:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAK7LNATZgJ4MxOFLUCNARWv3Zz=gpL-jGReDnBnArquiaXRWoQ@mail.gmail.com>

Hi Masahiro,

On Thu, 2018-05-10 at 18:16 +0900, Masahiro Yamada wrote:
> Hi.
> 
> 
> The previous thread was:
> "usb: dwc3: support clocks and resets for DWC3 core"
> https://patchwork.kernel.org/patch/10349623/
> 
> 
> I changed the subject because
> I think this is rather reset-control topic than USB.

Thank you for taking the time to write this up in such detail.

Indeed we were not expecting hardware that both defaults to deasserted
resets and does not have a power-on reset mechanism for the IP cores, so
yes, the framework currently assumes that all shared reset lines are
initially (or at some point in the past since power-on were) asserted.

I wonder if we are going to see this more often in the future, or
whether the Amlogic Meson SoCs will stay the exception.

> I am searching for a good way to initialize hardware devices
> in the following situation:
> 
>  - two or more hardware devices share the same reset line

So in general, at least during runtime, the driver must not expect reset
assertion to actually work, so the sharing device is not disturbed.

>  - those devices are not reset before booting the kernel
>    (i.e. flip flops in RTL are random state at driver probe)
> 
>  - the hardware IP is used by various SoCs,
>    and this issue only appears only on some.

Unfortunately for configurable IP cores like the DW ones I am not sure
to what extent these can actually be considered identical when
implemented in different SoCs.
I would expect that the hardware IP's basic requirements, like whether
the reset must be asserted at some defined point during operation, or
whether it has to be a pulse of da specific duration, does not change
between implementations.

> Specifically, the DWC3 USB IP case is in my mind,
> but this should apply to any hardware in general.
> 
> 
> 
> 
> Issue in detail
> ---------------
> 
> The DWC3 USB IP is very popular and used by various SoCs
> as you see in drivers/usb/dwc3/.
> 
> SoCs are using the same RTL as provided by Synopsys.
> (SoC vendors could tweak the RTL if they like,
> but it would rarely happen because it would just be troublesome)
> 
> So, let's assume we use the same IP with the same RTL version.
> It is *compatible* in terms of device tree.
> In this case, we should avoid differentiating the driver code per-SoC.
> 
> In fact, we ended up with
> commit ff0a632 ("usb: dwc3: of-simple: add support for shared and
> pulsed reset lines")
> commit e8284db ("usb: dwc3: of-simple: add support for the Amlogic
> Meson GXL and AXG SoCs")

I'm surprised, didn't all Meson SoCs gain level reset support for this
purpose?
I agree this should not be needed in the drivers, and I'd be very happy
if we could make this work on Meson with the dwc3 driver always
requesting shared resets.

> This means, the difference that comes from the reset _provider_
> must be implemented in the reset _consumer_.
> This is unfortunate.

I am not entirely happy about the shared vs. exclusive naming of the
API, but I have no better names. By choosing between the two, the
driver?states its requirements of the API, not whether the SoC it is
implemented in actually shares the reset line with other devices.

> So, I wonder if we can support it in sub-system level somehow
> instead of messing up the reset consumer driver.

I'd like to deflect and, hoping that this is an exception, ask whether
we could just do this in the driver?

>  - The reset topology is SoC-dependent.
>    A reset line is dedicated for single hardware for some SoCs,
>    and shared by multiple hardware for others.

In which case the driver must be able to cope with shared reset lines,
so it should use the shared reset API.

>  - The reset policy (pulse reset, level reset, or both of them)
>    are SoC-dependent (reset controller dependent).

While that may be the case for some hardware IP, I don't think it is the
case for the dwc3 driver. It doesn't need to assert the reset at any
time during normal operation, and it works with level resets. The
pulsed/exclusive reset handling really is just a workaround for the
missing power-on reset.

> RTL generally contains state machines.
> The initial state of flip flops are undefined on power-on
> hence the initial state of state machines are also undefined.
> 
> So, digital circuits needs explicit reset in general.
> 
> In some cases, the reset is performed before booting the kernel.
> 
>  - power-on reset
>    (FFs are put into defined state on power-on automatically)
> 
>  - a reset controller assert reset lines by default
>    (FFs are fixed to defined state.  If a reset consumer
>    driver deasserts the reset line, HW starts from the define state)
> 
>  - Firmware may reset the hardware before booting the kernel
> 
> 
> If nothing above is done, and the reset line is shared by multiple devices,
> we do not have a good way to put the hardware into the sane state.
> 
> 
> Reset consumers choose the required reset type:
> 
> - Shared reset:
>     reset_control_assert / reset_control_deassert work like
> clock_disable / clock_enable.
>     The consumer must be tolerant to the situation where the HW may
> not reset-asserted.
> 
> - Exclusive reset:
>     It is guaranteed reset_control_assert() really asserts the reset line,
>     but only one reset consumer grabs the reset line at the same time.

Yes. The last part is a limitation of the current framework
implementation, but one that I'd like to keep as long as possible.

To support shared resets with guaranteed assertion, we'd need a
notification framework with a possibility for drivers to temporarily
suspend their own operations to allow a reset request from a different
device, or to veto reset requests if that is not possible. And drivers
that try to reset need to properly handle failed or deferred reset
requests. That would introduce a level of complexity that I'd very much
like to avoid.

> As above, the state machines in digital circuits must start from a
> defined state.
> So, we want exclusive reset to reset the FFs.
> However, this is too much requirement
> since it is a reset line is often shared.

For missing power-on resets, what we really want is not "exclusive
reset", and not even "guaranteed reset assertion". We want that after
the initial reset_deassert, the reset line is deasserted and the device
has been in reset *at any point in the past* since power-on.

> How to save such an Amlogic case?

I think this case can be solved by either just initializing the reset
lines to asserted in the reset controller driver (in which case the
controller driver needs to know about which lines have to be asserted,
and needs to support assert/deassert ops) or by adding a fallback from
.deassert to .reset for reset controllers that don't have level reset
hardware support. That is, if the first driver calls
reset_control_deassert, the reset controller driver actually issues a
reset pulse.

> Hogging solve the problem?
> --------------------------
> 
> 
> I was thinking of a solution.
> 
> I may be missing something, but
> one solution might be reset hogging on the
> reset provider side.  This allows us to describe
> the initial state of reset lines in the reset controller.
> 
> The idea for "reset-hog" is similar to:
>  - "gpio-hog" defined in
>    Documentation/devicetree/bindings/gpio/gpio.txt
>  - "assigned-clocks" defined in
>    Documetation/devicetree/bindings/clock/clock-bindings.txt
> 
> 
> 
> For example,
> 
>    reset-controller {
>             ....
> 
>             line_a {
>                   reset-hog;
>                   resets = <1>;
>                   reset-assert;
>             };
>    }
> 
> 
> When the reset controller is registered,
> the reset ID '1' is asserted.
> 
> 
> So, all reset consumers that share the reset line '1'
> will start from the asserted state
> (i.e. defined state machine state).
> 
> 
> From the discussion with Martin Blumenstingl
> (https://lkml.org/lkml/2018/4/28/115),
> the problem for Amlogic is that
> the reset line is "de-asserted" by default.
> If so, the 'reset-hog' would fix the problem,
> and DWC3 driver would be able to use
> shared, level reset, I think.
> 
> 
> I think something may be missing from my mind,
> but I hope this will prompt the discussion,
> and we will find a better idea.

My question would is: do we need this complexity, or can this be solved
by just adding a workaround in the meson driver? If a lot of SoCs using
the simple reset driver had this requirement, I'd seriously consider
this, but it currently seems to me that this is just an issue with a
single SoC family.

regards
Philipp

^ permalink raw reply

* [reset-control] How to initialize hardware state with the shared reset line?
From: Philipp Zabel @ 2018-05-22 14:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAFBinCBTTmPqD0UP8zzMBjsjzP9fJx9_pmhGz0_Vy-V+wmhLXQ@mail.gmail.com>

Hi Martin,

On Mon, 2018-05-21 at 12:40 +0200, Martin Blumenstingl wrote:
> Hello,
> 
> On Mon, May 21, 2018 at 3:27 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> > Hi.
> > 
> > 
> > 2018-05-20 19:57 GMT+09:00 Martin Blumenstingl
> > <martin.blumenstingl@googlemail.com>:
> > > Hi,
> > > 
> > > On Thu, May 10, 2018 at 11:16 AM, Masahiro Yamada
> > > <yamada.masahiro@socionext.com> wrote:
> > > [snip]
> > > > I may be missing something, but
> > > > one solution might be reset hogging on the
> > > > reset provider side.  This allows us to describe
> > > > the initial state of reset lines in the reset controller.
> > > > 
> > > > The idea for "reset-hog" is similar to:
> > > >  - "gpio-hog" defined in
> > > >    Documentation/devicetree/bindings/gpio/gpio.txt
> > > >  - "assigned-clocks" defined in
> > > >    Documetation/devicetree/bindings/clock/clock-bindings.txt
> > > > 
> > > > 
> > > > 
> > > > For example,
> > > > 
> > > >    reset-controller {
> > > >             ....
> > > > 
> > > >             line_a {
> > > >                   reset-hog;
> > > >                   resets = <1>;
> > > >                   reset-assert;
> > > >             };
> > > >    }
> > > > 
> > > > 
> > > > When the reset controller is registered,
> > > > the reset ID '1' is asserted.
> > > > 
> > > > 
> > > > So, all reset consumers that share the reset line '1'
> > > > will start from the asserted state
> > > > (i.e. defined state machine state).
> > > 
> > > I wonder if a "reset hog" can be board specific:
> > > - GPIO hogs are definitely board specific (meson-gxbb-odroidc2.dts for
> > > example uses it to take the USB hub out of reset)
> > > - assigned-clock-parents (and the like) can also be board specific (I
> > > made up a use-case since I don't know of any actual examples: board A
> > > uses an external XTAL while board B uses some other internal
> > > clock-source because it doesn't have an external XTAL)
> > > 
> > > however, can reset lines be board specific? or in other words: do we
> > > need to describe them in device-tree?
> > 
> > Indeed.
> > 
> > I did not come up with board-specific cases.
> > 
> > The problem we are discussing is SoC-specific,
> > and reset-controller drivers are definitely SoC-specific.
> > 
> > So, I think the initial state can be coded in drivers instead of DT.
> 
> OK, let's also hear Philipp's (reset framework maintainer) opinion on this

I'd like to know if there are other SoC families besides Amlogic Meson
that potentially could have this problem and about how many of the
resets that are documented in include/dt-bindings/reset/amlogic,meson*
we are actually talking. Are all of those initially deasserted and none
of the connected peripherals have power-on reset mechanisms?

> > > we could extend struct reset_controller_dev (= reset controller
> > > driver) if they are not board specific:
> > > - either assert all reset lines by default except if they are listed
> > > in a new field (may break backwards compatibility, requires testing of
> > > all reset controller drivers)
> > 
> > This is quite simple, but I am afraid there are some cases where the forcible
> > reset-assert is not preferred.
> > 
> > For example, the earlycon.  When we use earlycon, we would expect it has been
> > initialized by a boot-loader, or something.
> > If it is reset-asserted on the while, the console output
> > will not be good.
> 
> indeed, so let's skip this idea

Maybe we should at first add initial reset assertion to the Meson driver
on a case by case bases?
We can't add required reset hog DT bindings to the Meson reset
controller anyway without breaking DT backwards compatibility.

> > > - specify a list of reset lines and their desired state (or to keep it
> > > easy: specify a list of reset lines that should be asserted)
> > > (I must admit that this is basically your idea but the definition is
> > > moved from device-tree to the reset controller driver)
> > 
> > Yes, I think the list of "reset line ID" and "init state" pairs
> > would be nicer.
> 
> $ grep -R "of_reset_n_cells = [^1]" drivers/reset/
> drivers/reset/reset-berlin.c:   priv->rcdev.of_reset_n_cells = 2;
> drivers/reset/hisilicon/reset-hi3660.c: rc->rst.of_reset_n_cells = 2;
> drivers/reset/reset-ti-sci.c:   data->rcdev.of_reset_n_cells = 2;
> drivers/reset/reset-lantiq.c:   priv->rcdev.of_reset_n_cells = 2;
> 
> everything else uses only one reset cell
> from the lantiq reset dt-binding documentation: "The first cell takes
> the reset set bit and the second cell takes the status bit."
> 
> I'm not sure what to do with drivers that specify != 1 reset-cell
> though if we use a simple "init state pair"
> maybe Philipp can share his opinion on this one as well

See above, so far I am not convinced (either way) whether this should be
described in the DT at all.

> > > any "chip" specific differences could be expressed by using a
> > > different of_device_id
> > > 
> > > one the other hand: your "reset hog" solution looks fine to me if
> > > reset lines can be board specific
> > > 
> > > > From the discussion with Martin Blumenstingl
> > > > (https://lkml.org/lkml/2018/4/28/115),
> > > > the problem for Amlogic is that
> > > > the reset line is "de-asserted" by default.
> > > > If so, the 'reset-hog' would fix the problem,
> > > > and DWC3 driver would be able to use
> > > > shared, level reset, I think.
> > > 
> > > I think you are right: if we could control the initial state then we
> > > should be able to use level resets
> > 
> > 
> > Even further, can we drop the shared reset_control_reset() support, maybe?
> > (in other words, revert commit 7da33a37b48f11)
> 
> I believe we need to keep this if there's hardware out there:
> - where the reset controller only supports reset pulses
> - at least one reset line is shared between multiple devices
> 
> I didn't have a closer look at the Amlogic Meson6 SoC yet, but I think
> it matches above criteria. as far as I know:
> - the USB situation there is similar to Meson8b (USB controllers and
> PHYs share a reset line)
> - it uses an older reset controller IP block which does not support
> level resets (only reset pulses)

See my answer to Masahiro's first mail. I think somebody suggested in
the past to add a fallback from the deassert to the reset op. I think
this is something that should work in this case.

regards
Philipp

^ permalink raw reply

* [PATCH RFC V2 2/6] hwmon: Add support for RPi voltage sensor
From: Guenter Roeck @ 2018-05-22 14:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <659923372.11518.1526997096662@email.1und1.de>

On 05/22/2018 06:51 AM, Stefan Wahren wrote:
> Hi Guenter,
> 
>> Guenter Roeck <linux@roeck-us.net> hat am 22. Mai 2018 um 15:41 geschrieben:
>>
>>
>> On 05/22/2018 04:21 AM, Stefan Wahren wrote:
>>> Currently there is no easy way to detect undervoltage conditions on a
>>> remote Raspberry Pi. This hwmon driver retrieves the state of the
>>> undervoltage sensor via mailbox interface. The handling based on
>>> Noralf's modifications to the downstream firmware driver. In case of
>>> an undervoltage condition only an entry is written to the kernel log.
>>>
>>> CC: "Noralf Tr?nnes" <noralf@tronnes.org>
>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>>> ---
>>>    Documentation/hwmon/raspberrypi-hwmon |  22 +++++
>>>    drivers/hwmon/Kconfig                 |  10 ++
>>>    drivers/hwmon/Makefile                |   1 +
>>>    drivers/hwmon/raspberrypi-hwmon.c     | 168 ++++++++++++++++++++++++++++++++++
>>>    4 files changed, 201 insertions(+)
>>>    create mode 100644 Documentation/hwmon/raspberrypi-hwmon
>>>    create mode 100644 drivers/hwmon/raspberrypi-hwmon.c
>>>
>>> diff --git a/Documentation/hwmon/raspberrypi-hwmon b/Documentation/hwmon/raspberrypi-hwmon
>>> new file mode 100644
>>> index 0000000..3c92e2c
>>> --- /dev/null
>>> +++ b/Documentation/hwmon/raspberrypi-hwmon
>>> @@ -0,0 +1,22 @@
>>> +Kernel driver raspberrypi-hwmon
>>> +===============================
>>> +
>>> +Supported boards:
>>> +  * Raspberry Pi A+ (via GPIO on SoC)
>>> +  * Raspberry Pi B+ (via GPIO on SoC)
>>> +  * Raspberry Pi 2 B (via GPIO on SoC)
>>> +  * Raspberry Pi 3 B (via GPIO on port expander)
>>> +  * Raspberry Pi 3 B+ (via PMIC)
>>> +
>>> +Author: Stefan Wahren <stefan.wahren@i2se.com>
>>> +
>>> +Description
>>> +-----------
>>> +
>>> +This driver periodically polls a mailbox property of the VC4 firmware to detect
>>> +undervoltage conditions.
>>> +
>>> +Sysfs entries
>>> +-------------
>>> +
>>> +in0_lcrit_alarm		Undervoltage alarm
>>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>>> index 768aed5..9a5bdb0 100644
>>> --- a/drivers/hwmon/Kconfig
>>> +++ b/drivers/hwmon/Kconfig
>>> @@ -1298,6 +1298,16 @@ config SENSORS_PWM_FAN
>>>    	  This driver can also be built as a module.  If so, the module
>>>    	  will be called pwm-fan.
>>>    
>>> +config SENSORS_RASPBERRYPI_HWMON
>>> +	tristate "Raspberry Pi voltage monitor"
>>> +	depends on RASPBERRYPI_FIRMWARE || COMPILE_TEST
>>> +	help
>>> +	  If you say yes here you get support for voltage sensor on the
>>> +	  Raspberry Pi.
>>> +
>>> +	  This driver can also be built as a module. If so, the module
>>> +	  will be called raspberrypi-hwmon.
>>> +
>>>    config SENSORS_SHT15
>>>    	tristate "Sensiron humidity and temperature sensors. SHT15 and compat."
>>>    	depends on GPIOLIB || COMPILE_TEST
>>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>>> index e7d52a3..a929770 100644
>>> --- a/drivers/hwmon/Makefile
>>> +++ b/drivers/hwmon/Makefile
>>> @@ -141,6 +141,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
>>>    obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
>>>    obj-$(CONFIG_SENSORS_POWR1220)  += powr1220.o
>>>    obj-$(CONFIG_SENSORS_PWM_FAN)	+= pwm-fan.o
>>> +obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON)	+= raspberrypi-hwmon.o
>>>    obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
>>>    obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
>>>    obj-$(CONFIG_SENSORS_SCH5627)	+= sch5627.o
>>> diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
>>> new file mode 100644
>>> index 0000000..6233e84
>>> --- /dev/null
>>> +++ b/drivers/hwmon/raspberrypi-hwmon.c
>>> @@ -0,0 +1,168 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Raspberry Pi voltage sensor driver
>>> + *
>>> + * Based on firmware/raspberrypi.c by Noralf Tr?nnes
>>> + *
>>> + * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
>>> + */
>>> +#include <linux/device.h>
>>> +#include <linux/err.h>
>>> +#include <linux/hwmon.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/workqueue.h>
>>> +#include <soc/bcm2835/raspberrypi-firmware.h>
>>> +
>>> +#define UNDERVOLTAGE_STICKY_BIT	BIT(16)
>>> +
>>> +struct rpi_hwmon_data {
>>> +	struct device *hwmon_dev;
>>> +	struct rpi_firmware *fw;
>>> +	u32 last_throttled;
>>> +	struct delayed_work get_values_poll_work;
>>> +};
>>> +
>>> +static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
>>> +{
>>> +	u32 new_uv, old_uv, value;
>>> +	int ret;
>>> +
>>> +	/* Request firmware to clear sticky bits */
>>> +	value = 0xffff;
>>> +
>>> +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
>>> +				    &value, sizeof(value));
>>> +	if (ret) {
>>> +		dev_err_once(data->hwmon_dev, "Failed to get throttled (%d)\n",
>>> +			     ret);
>>> +		return;
>>> +	}
>>> +
>>> +	new_uv = value & UNDERVOLTAGE_STICKY_BIT;
>>> +	old_uv = data->last_throttled & UNDERVOLTAGE_STICKY_BIT;
>>> +	data->last_throttled = value;
>>> +
>>> +	if (new_uv == old_uv)
>>> +		return;
>>> +
>>> +	if (new_uv)
>>> +		dev_crit(data->hwmon_dev, "Undervoltage detected!\n");
>>> +	else
>>> +		dev_info(data->hwmon_dev, "Voltage normalised\n");
>>> +
>>> +	sysfs_notify(&data->hwmon_dev->kobj, NULL, "in0_lcrit_alarm");
>>> +}
>>> +
>>> +static void get_values_poll(struct work_struct *work)
>>> +{
>>> +	struct rpi_hwmon_data *data;
>>> +
>>> +	data = container_of(work, struct rpi_hwmon_data,
>>> +			    get_values_poll_work.work);
>>> +
>>> +	rpi_firmware_get_throttled(data);
>>> +
>>> +	/*
>>> +	 * We can't run faster than the sticky shift (100ms) since we get
>>> +	 * flipping in the sticky bits that are cleared.
>>> +	 */
>>> +	schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
>>> +}
>>> +
>>> +static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
>>> +		    u32 attr, int channel, long *val)
>>> +{
>>> +	struct rpi_hwmon_data *data = dev_get_drvdata(dev);
>>> +
>>> +	*val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
>>> +	return 0;
>>> +}
>>> +
>>> +static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
>>> +			      u32 attr, int channel)
>>> +{
>>> +	return 0444;
>>> +}
>>> +
>>> +static const u32 rpi_in_config[] = {
>>> +	HWMON_I_LCRIT_ALARM,
>>> +	0
>>> +};
>>> +
>>> +static const struct hwmon_channel_info rpi_in = {
>>> +	.type = hwmon_in,
>>> +	.config = rpi_in_config,
>>> +};
>>> +
>>> +static const struct hwmon_channel_info *rpi_info[] = {
>>> +	&rpi_in,
>>> +	NULL
>>> +};
>>> +
>>> +static const struct hwmon_ops rpi_hwmon_ops = {
>>> +	.is_visible = rpi_is_visible,
>>> +	.read = rpi_read,
>>> +};
>>> +
>>> +static const struct hwmon_chip_info rpi_chip_info = {
>>> +	.ops = &rpi_hwmon_ops,
>>> +	.info = rpi_info,
>>> +};
>>> +
>>> +static int rpi_hwmon_probe(struct platform_device *pdev)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct rpi_hwmon_data *data;
>>> +	int ret;
>>> +
>>> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>>> +	if (!data)
>>> +		return -ENOMEM;
>>> +
>>> +	data->fw = platform_get_drvdata(to_platform_device(dev->parent));
>>> +	if (!data->fw)
>>> +		return -EPROBE_DEFER;
>>> +
>>
>> I am a bit at loss here (and sorry I didn't bring this up before).
>> How would this ever be possible, given that the driver is registered
>> from the firmware driver ?
> 
> Do you refer to the (wrong) return code, the assumption that the parent must be a platform driver or a possible race?
> 

The return code is one thing. My question was how the driver would ever be instantiated
with platform_get_drvdata(to_platform_device(dev->parent)) == NULL (but dev->parent != NULL),
so I referred to the race. But, sure, a second question would be how that would indicate
that the parent is not instantiated yet (which by itself seems like an odd question).

Yet another question, as you point out, is why to use platform_get_drvdata(to_platform_device(dev->parent))
instead of dev_get_drvdata(dev->parent).

Guenter

>>
>>> +	ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
>>> +				    &data->last_throttled,
>>> +				    sizeof(data->last_throttled));
>>> +	if (ret)
>>> +		return -ENODEV;
>>> +
>>> +	data->hwmon_dev = devm_hwmon_device_register_with_info(dev, "rpi_volt",
>>> +							       data,
>>> +							       &rpi_chip_info,
>>> +							       NULL);
>>> +
>>> +	INIT_DELAYED_WORK(&data->get_values_poll_work, get_values_poll);
>>> +	platform_set_drvdata(pdev, data);
>>> +
>>> +	if (!PTR_ERR_OR_ZERO(data->hwmon_dev))
>>> +		schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
>>> +
>>> +	return PTR_ERR_OR_ZERO(data->hwmon_dev);
>>> +}
>>> +
>>> +static int rpi_hwmon_remove(struct platform_device *pdev)
>>> +{
>>> +	struct rpi_hwmon_data *data = platform_get_drvdata(pdev);
>>> +
>>> +	cancel_delayed_work_sync(&data->get_values_poll_work);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct platform_driver rpi_hwmon_driver = {
>>> +	.probe = rpi_hwmon_probe,
>>> +	.remove = rpi_hwmon_remove,
>>> +	.driver = {
>>> +		.name = "raspberrypi-hwmon",
>>> +	},
>>> +};
>>> +module_platform_driver(rpi_hwmon_driver);
>>> +
>>> +MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
>>> +MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
>>> +MODULE_LICENSE("GPL v2");
>>>
>>
> 

^ permalink raw reply

* [RFC PATCH v2] arm64: fault: Don't leak data in ESR context for user fault on kernel VA
From: Dave Martin @ 2018-05-22 14:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAFEAcA9i4_neKwaWPm_gV=fAgASL4B9X8Z++c+NvGA+JYSdDZA@mail.gmail.com>

On Tue, May 22, 2018 at 02:48:29PM +0100, Peter Maydell wrote:
> On 22 May 2018 at 14:38, Will Deacon <will.deacon@arm.com> wrote:
> > Hi Peter,
> >
> > Sorry for the delay in getting to this! Comments inline.
> >
> > On Thu, Apr 19, 2018 at 04:48:33PM +0100, Peter Maydell wrote:
> 
> >> +                     /*
> >> +                      * These bits provide only information about the
> >> +                      * faulting instruction, which userspace knows already.
> >> +                      * We explicitly clear bits which are architecturally
> >> +                      * RES0 in case they are given meanings in future.
> >> +                      */
> >> +                     if (esr & ESR_ELx_ISV)
> >> +                             esr &= ESR_ELx_EC_MASK | ESR_ELx_IL |
> >> +                                     ESR_ELx_ISV | ESR_ELx_SAS |
> >> +                                     ESR_ELx_SSE | ESR_ELx_SRT_MASK |
> >> +                                     ESR_ELx_SF | ESR_ELx_AR | ESR_ELx_CM |
> >> +                                     ESR_ELx_WNR;
> >
> > Reading through the ARM ARM, it seems to say that ISV is always 0 for
> > faults reported in ESR_EL1, which implies we can drop ISV, SAS, SSE, SRT,
> > SF and AR from this list and actually drop the conditional altogether.
> 
> Mmm, I guess so, if we're guaranteed to only be working with ESRs
> taken to EL1 (or we want to present userspace with an ESR that

There is no direct interface between EL0 and EL2 with the stage2
translation enabled, so even if this data is available for a fault at
EL2, we won't be signalling the fault via delivering a signal to EL0.

> looks like that regardless of what EL we took it to). I'll respin
> without the conditional.

Sounds fair.  It might have been me that suggested this list of fields
in the first place: I'd not completely understood the ISV behaviour
previously, it seems.

Cheers
---Dave

^ permalink raw reply

* [PATCH v2] ARM64: dts: sun50i: a64: Add spi flash node for sopine
From: Maxime Ripard @ 2018-05-22 14:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180521115413.92070-1-manu@freebsd.org>

On Mon, May 21, 2018 at 01:54:13PM +0200, Emmanuel Vadot wrote:
> The Sopine and Pine64-LTS have a winbond w25q128 spi flash on spi0.
> Add a node for it.
> 
> Signed-off-by: Emmanuel Vadot <manu@freebsd.org>

Fixed the prefix and applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/bf0e2273/attachment.sig>

^ permalink raw reply

* [PATCH v2] ARM64: dts: sun50i: h5: Add spi flash node for OrangePi PC2
From: Maxime Ripard @ 2018-05-22 14:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180521115426.92115-1-manu@freebsd.org>

On Mon, May 21, 2018 at 01:54:26PM +0200, Emmanuel Vadot wrote:
> The OrangePi PC2 have an mx25l1606e spi flash.
> Add a node for it.
> 
> Signed-off-by: Emmanuel Vadot <manu@freebsd.org>

Fixed the prefix and applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/3426bebf/attachment-0001.sig>

^ permalink raw reply

* [PATCH 1/9] ARM: shmobile: defconfig: Enable MTD command line partition parsing
From: Geert Uytterhoeven @ 2018-05-22 14:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-1-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> In preparation for removing MTD partitioning from the DTs and moving
> it over to kernel command line partition parsing, enable the support
> for kernel command line MTD partition parsing.
>
> The argument for not having MTD partitions in the DT is the same as
> for not having hard drive partitions in DT, neither describes the
> hardware itself, so it shouldn't be in the DT. Furthermore, kernel
> command line MTD partition passing allows greater flexibility in
> case someone decided to repartition the flash, which is well in the
> realm of possibility with these systems.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] arm64: Kconfig: Enable LSE atomics by default
From: Marc Zyngier @ 2018-05-22 14:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526926462-19214-1-git-send-email-will.deacon@arm.com>

On 21/05/18 19:14, Will Deacon wrote:
> Now that we're seeing CPUs shipping with LSE atomics, default them to
> 'on' in Kconfig. CPUs without the instructions will continue to use
> LDXR/STXR-based sequences, but they will be placed out-of-line by the
> compiler.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH 4/9] ARM: dts: koelsch: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-4-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:512k at 0(loader),5632k(user),-(flash)
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [RFC PATCH v2] arm64: fault: Don't leak data in ESR context for user fault on kernel VA
From: Peter Maydell @ 2018-05-22 14:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522141146.GF13470@e103592.cambridge.arm.com>

On 22 May 2018 at 15:11, Dave Martin <Dave.Martin@arm.com> wrote:
> On Tue, May 22, 2018 at 02:48:29PM +0100, Peter Maydell wrote:
>> On 22 May 2018 at 14:38, Will Deacon <will.deacon@arm.com> wrote:
>> > Hi Peter,
>> >
>> > Sorry for the delay in getting to this! Comments inline.
>> >
>> > On Thu, Apr 19, 2018 at 04:48:33PM +0100, Peter Maydell wrote:
>>
>> >> +                     /*
>> >> +                      * These bits provide only information about the
>> >> +                      * faulting instruction, which userspace knows already.
>> >> +                      * We explicitly clear bits which are architecturally
>> >> +                      * RES0 in case they are given meanings in future.
>> >> +                      */
>> >> +                     if (esr & ESR_ELx_ISV)
>> >> +                             esr &= ESR_ELx_EC_MASK | ESR_ELx_IL |
>> >> +                                     ESR_ELx_ISV | ESR_ELx_SAS |
>> >> +                                     ESR_ELx_SSE | ESR_ELx_SRT_MASK |
>> >> +                                     ESR_ELx_SF | ESR_ELx_AR | ESR_ELx_CM |
>> >> +                                     ESR_ELx_WNR;
>> >
>> > Reading through the ARM ARM, it seems to say that ISV is always 0 for
>> > faults reported in ESR_EL1, which implies we can drop ISV, SAS, SSE, SRT,
>> > SF and AR from this list and actually drop the conditional altogether.
>>
>> Mmm, I guess so, if we're guaranteed to only be working with ESRs
>> taken to EL1 (or we want to present userspace with an ESR that
>
> There is no direct interface between EL0 and EL2 with the stage2
> translation enabled, so even if this data is available for a fault at
> EL2, we won't be signalling the fault via delivering a signal to EL0.

Right -- and even if we did somehow end up with that, we probably
wouldn't want to leak to userspace that their access was trapped
to EL2 rather than EL1, so we should present the illusion that
it was trapped at EL1 regardless.

thanks
-- PMM

^ permalink raw reply

* [PATCH v3] arm64: allwinner: a64: Add Amarula A64-Relic initial support
From: Maxime Ripard @ 2018-05-22 14:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522132228.6564-1-jagan@amarulasolutions.com>

On Tue, May 22, 2018 at 06:52:28PM +0530, Jagan Teki wrote:
> Amarula A64-Relic is Allwinner A64 based IoT device, which support
> - Allwinner A64 Cortex-A53
> - Mali-400MP2 GPU
> - AXP803 PMIC
> - 1GB DDR3 RAM
> - 8GB eMMC
> - AP6330 Wifi/BLE
> - MIPI-DSI
> - CSI: OV5640 sensor
> - USB OTG

You claim that this is doing OTG...

[..]

> +&usb_otg {
> +	dr_mode = "peripheral";
> +	status = "okay";
> +};

... and yet you're setting it as peripheral...

> +&usbphy {
> +	usb0_id_det-gpios = <&pio 7 9 GPIO_ACTIVE_HIGH>; /* PH9 */
> +	usb0_vbus-supply = <&reg_drivevbus>;

While you have an ID pin and a controllable VBUS. Which one is it?

Maxime
-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/f71e03b3/attachment-0001.sig>

^ permalink raw reply

* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Jon Hunter @ 2018-05-22 14:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526639490-12167-9-git-send-email-ulf.hansson@linaro.org>

Hi Ulf,

On 18/05/18 11:31, Ulf Hansson wrote:
> To support devices being partitioned across multiple PM domains, let's
> start by extending genpd to cope with these configurations.
> 
> More precisely, add a new exported function, genpd_dev_pm_attach_by_id(),
> similar to genpd_dev_pm_attach(), but the new function also allows the
> caller to provide an index to what PM domain it wants to attach.
> 
> Furthermore, let genpd register a new virtual struct device via calling
> device_register() and attach it to the corresponding PM domain, which is
> looked up via calling the existing genpd OF functions. Note that the new
> device is needed, because only one PM domain can be attached per device. At
> successful attachment, genpd_dev_pm_attach_by_id() returns the new device,
> allowing the caller to operate on it to deal with power management.
> 
> To deal with detaching of a PM domain for multiple PM domain case, we can
> still re-use the existing genpd_dev_pm_detach() function, although we need
> to extend it to cover cleanup of the earlier registered device, via calling
> device_unregister().
> 
> An important note, genpd_dev_pm_attach_by_id() shall only be called by the
> driver core / PM core, similar to how genpd_dev_pm_attach() is used.
> Following changes deploys this.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>   drivers/base/power/domain.c | 79 +++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/pm_domain.h   |  8 +++++
>   2 files changed, 87 insertions(+)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index d538640..ffeb6ea 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2171,6 +2171,15 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>   }
>   EXPORT_SYMBOL_GPL(of_genpd_remove_last);
>   
> +static void genpd_release_dev(struct device *dev)
> +{
> +	kfree(dev);
> +}
> +
> +static struct bus_type genpd_bus_type = {
> +	.name		= "genpd",
> +};
> +
>   /**
>    * genpd_dev_pm_detach - Detach a device from its PM domain.
>    * @dev: Device to detach.
> @@ -2208,6 +2217,10 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off)
>   
>   	/* Check if PM domain can be powered off after removing this device. */
>   	genpd_queue_power_off_work(pd);
> +
> +	/* Unregister the device if it was created by genpd. */
> +	if (dev->bus == &genpd_bus_type)
> +		device_unregister(dev);
>   }
>   
>   static void genpd_dev_pm_sync(struct device *dev)
> @@ -2298,6 +2311,66 @@ int genpd_dev_pm_attach(struct device *dev)
>   }
>   EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>   
> +/**
> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
> + * @dev: Device to attach.
> + * @index: The index of the PM domain.
> + *
> + * Parse device's OF node to find a PM domain specifier at the provided @index.
> + * If such is found, allocates a new device and attaches it to retrieved
> + * pm_domain ops.
> + *
> + * Returns the allocated device if successfully attached PM domain, NULL when
> + * the device don't need a PM domain or have a single PM domain, else PTR_ERR()
> + * in case of failures. Note that if a power-domain exists for the device, but
> + * cannot be found or turned on, then return PTR_ERR(-EPROBE_DEFER) to ensure
> + * that the device is not probed and to re-try again later.
> + */
> +struct device *genpd_dev_pm_attach_by_id(struct device *dev,
> +					 unsigned int index)
> +{
> +	struct device *genpd_dev;
> +	int num_domains;
> +	int ret;
> +
> +	if (!dev->of_node)
> +		return NULL;
> +
> +	/* Deal only with devices using multiple PM domains. */
> +	num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
> +						 "#power-domain-cells");
> +	if (num_domains < 2 || index >= num_domains)
> +		return NULL;
> +
> +	/* Allocate and register device on the genpd bus. */
> +	genpd_dev = kzalloc(sizeof(*genpd_dev), GFP_KERNEL);
> +	if (!genpd_dev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	dev_set_name(genpd_dev, "genpd:%u:%s", index, dev_name(dev));
> +	genpd_dev->bus = &genpd_bus_type;
> +	genpd_dev->release = genpd_release_dev;
> +
> +	ret = device_register(genpd_dev);
> +	if (ret) {
> +		kfree(genpd_dev);
> +		return ERR_PTR(ret);
> +	}
> +
> +	/* Try to attach the device to the PM domain at the specified index. */
> +	ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index);
> +	if (ret < 1) {
> +		device_unregister(genpd_dev);
> +		return ret ? ERR_PTR(ret) : NULL;
> +	}
> +
> +	pm_runtime_set_active(genpd_dev);
> +	pm_runtime_enable(genpd_dev);
> +
> +	return genpd_dev;
> +}
> +EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_by_id);

Thanks for sending this. Believe it or not this has still been on my to-do list
and so we definitely need a solution for Tegra.

Looking at the above it appears that additional power-domains exposed as devices
to the client device. So I assume that this means that the drivers for devices
with multiple power-domains will need to call RPM APIs for each of these
additional power-domains. Is that correct?

If so, I can see that that would work, but it does not seem to fit the RPM model
where currently for something like device clocks, the RPM callbacks can handle
all clocks at once.

I was wondering why we could not add a list of genpd domains to the
dev_pm_domain struct for the device? For example ...

diff --git a/include/linux/pm.h b/include/linux/pm.h
index e723b78d8357..a11d6db3c077 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -659,6 +659,7 @@ extern void dev_pm_put_subsys_data(struct device *dev);
  * subsystem-level and driver-level callbacks.
  */
 struct dev_pm_domain {
+       struct list_head        genpd_list;
        struct dev_pm_ops       ops;
        void (*detach)(struct device *dev, bool power_off);
        int (*activate)(struct device *dev);
@@ -666,6 +667,11 @@ struct dev_pm_domain {
        void (*dismiss)(struct device *dev);
 };

+struct dev_pm_domain_link {
+       struct generic_pm_domain *genpd;
+       struct list_head node;
+};
+
 /*
  * The PM_EVENT_ messages are also used by drivers implementing the legacy
  * suspend framework, based on the ->suspend() and ->resume() callbacks common
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index e61b5cd79fe2..019593804670 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -51,7 +51,6 @@ struct dev_pm_opp;

 struct generic_pm_domain {
        struct device dev;
-       struct dev_pm_domain domain;    /* PM domain operations */
        struct list_head gpd_list_node; /* Node in the global PM domains list */
        struct list_head master_links;  /* Links with PM domain as a master */
        struct list_head slave_links;   /* Links with PM domain as a slave */
@@ -99,11 +98,6 @@ struct generic_pm_domain {

 };

-static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
-{
-       return container_of(pd, struct generic_pm_domain, domain);
-}
-

Obviously the above will not compile but the intent would be to allocate a
dev_pm_domain_link struct per power-domain that the device needs and add
to the genpd_list for the device. It would be a much bigger change in
having to iterate through all the power-domains when turning devices on
and off, however, it would simplify the client driver. 

Cheers
Jon

-- 
nvpublic

^ permalink raw reply related

* [PATCH 2/9] ARM: dts: lager: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-2-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)

I guess s/4096k/4m/ works, too?

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 3/9] ARM: dts: stout: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-3-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:512k at 0(loader),256k(uboot),256k(uboot-env),-(flash)
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 5/9] ARM: dts: porter: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-5-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:256k at 0(loader_prg),4096k(user_prg),-(flash_fs)

4m?

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 6/9] ARM: dts: wheat: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-6-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)

I think the "@0" can be dropped, as it's optional?
4m?

(Gaining more knowledge during reviewing ;-)

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 7/9] ARM: dts: gose: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-7-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)

Drop @0? 4m?

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 8/9] ARM: dts: alt: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-8-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:256k at 0(loader),256k(system),-(user)

Drop @0?

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox