* Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
From: Prakhar Srivastava @ 2020-05-18 20:16 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, linux-arm-kernel, linux-kernel, linuxppc-dev,
devicetree, linux-integrity, linux-security-module,
catalin.marinas, will, mpe, benh, paulus, frowand.list, zohar,
dmitry.kasatkin, jmorris, serge, pasha.tatashin, allison,
kstewart, takahiro.akashi, tglx, vincenzo.frascino, masahiroy,
james.morse, bhsharma, mbrugger, hsinyi, tao.li, christophe.leroy,
gregkh, nramas, tusharsu, balajib
In-Reply-To: <20200512230509.GA2654@bogus>
On 5/12/20 4:05 PM, Rob Herring wrote:
> On Wed, May 06, 2020 at 10:50:04PM -0700, Prakhar Srivastava wrote:
>> Hi Mark,
>
> Please don't top post.
>
>> This patch set currently only address the Pure DT implementation.
>> EFI and ACPI implementations will be posted in subsequent patchsets.
>>
>> The logs are intended to be carried over the kexec and once read the
>> logs are no longer needed and in prior conversation with James(
>> https://lore.kernel.org/linux-arm-kernel/0053eb68-0905-4679-c97a-00c5cb6f1abb@arm.com/)
>> the apporach of using a chosen node doesn't
>> support the case.
>>
>> The DT entries make the reservation permanent and thus doesnt need kernel
>> segments to be used for this, however using a chosen-node with
>> reserved memory only changes the node information but memory still is
>> reserved via reserved-memory section.
>
> I think Mark's point was whether it needs to be permanent. We don't
> hardcode the initrd address for example.
>
Thankyou for clarifying my misunderstanding, i am modelling this keeping
to the TPM log implementation that uses a reserved memory. I will rev up
the version with chosen-node support.
That will make the memory reservation free after use.
>> On 5/5/20 2:59 AM, Mark Rutland wrote:
>>> Hi Prakhar,
>>>
>>> On Mon, May 04, 2020 at 01:38:27PM -0700, Prakhar Srivastava wrote:
>>>> IMA during kexec(kexec file load) verifies the kernel signature and measures
>
> What's IMAIMA is a LSM attempting to detect if files have been accidentally or
maliciously altered, both remotely and locally, it can also be used
to appraise a file's measurement against a "good" value stored as an
extended attribute, and enforce local file integrity.
IMA also validates and measures the signers of the kernel and initrd
during kexec. The measurements are extended to PCR 10(configurable) and
the logs stored in memory, however once kexec'd the logs are lost. Kexec
is used as secondary boot loader in may use cases and loosing the signer
creates a security hole.
This patch is an implementation to carry over the logs and making it
possible to remotely validate the signers of the kernel and initrd. Such
a support exits only in powerpc.
This patch makes the carry over of logs architecture independent and
puts the complexity in a driver.
Thanks,
Prakhar
>
>>>> the signature of the kernel. The signature in the logs can be used to verfiy the
>>>> authenticity of the kernel. The logs don not get carried over kexec and thus
>>>> remote attesation cannot verify the signature of the running kernel.
>>>>
>>>> Introduce an ABI to carry forward the ima logs over kexec.
>>>> Memory reserved via device tree reservation can be used to store and read
>>>> via the of_* functions.
>>>
>>> This flow needs to work for:
>>>
>>> 1) Pure DT
>>> 2) DT + EFI memory map
>>> 3) ACPI + EFI memory map
>>>
>>> ... and if this is just for transiently passing the log, I don't think
>>> that a reserved memory region is the right thing to use, since they're
>>> supposed to be more permanent.
>>>
>>> This sounds analogous to passing the initrd, and should probably use
>>> properties under the chosen node (which can be used for all three boot
>>> flows above).
>>>
>>> For reference, how big is the IMA log likely to be? Does it need
>>> physically contiguous space?
>>
>> It purely depends on the policy used and the modules/files that are accessed
>> for my local testing over a kexec session the log in
>> about 30KB.
>>
>> Current implementation expects enough contiguous memory to allocated to
>> carry forward the logs. If the log size exceeds the reserved memory the
>> call will fail.
>>
>> Thanks,
>> Prakhar Srivastava
>>>
>>> Thanks,
>>> Mark.
>>>
>>>>
>>>> Reserved memory stores the size(sizeof(size_t)) of the buffer in the starting
>>>> address, followed by the IMA log contents.
>>>>
>>>> Tested on:
>>>> arm64 with Uboot
>>>>
>>>> Prakhar Srivastava (2):
>>>> Add a layer of abstraction to use the memory reserved by device tree
>>>> for ima buffer pass.
>>>> Add support for ima buffer pass using reserved memory for arm64 kexec.
>>>> Update the arch sepcific code path in kexec file load to store the
>>>> ima buffer in the reserved memory. The same reserved memory is read
>>>> on kexec or cold boot.
>>>>
>>>> arch/arm64/Kconfig | 1 +
>>>> arch/arm64/include/asm/ima.h | 22 ++++
>>>> arch/arm64/include/asm/kexec.h | 5 +
>>>> arch/arm64/kernel/Makefile | 1 +
>>>> arch/arm64/kernel/ima_kexec.c | 64 ++++++++++
>>>> arch/arm64/kernel/machine_kexec_file.c | 1 +
>>>> arch/powerpc/include/asm/ima.h | 3 +-
>>>> arch/powerpc/kexec/ima.c | 14 ++-
>>>> drivers/of/Kconfig | 6 +
>>>> drivers/of/Makefile | 1 +
>>>> drivers/of/of_ima.c | 165 +++++++++++++++++++++++++
>>>> include/linux/of.h | 34 +++++
>>>> security/integrity/ima/ima_kexec.c | 15 ++-
>>>> 13 files changed, 325 insertions(+), 7 deletions(-)
>>>> create mode 100644 arch/arm64/include/asm/ima.h
>>>> create mode 100644 arch/arm64/kernel/ima_kexec.c
>>>> create mode 100644 drivers/of/of_ima.c
>>>>
>>>> --
>>>> 2.25.1
>>>>
^ permalink raw reply
* Re: [PATCH 3/3] power: supply: bq25150 introduce the bq25150
From: Ricardo Rivera-Matos @ 2020-05-18 19:59 UTC (permalink / raw)
To: Andrew F. Davis, Dan Murphy, sre
Cc: sspatil, linux-pm, robh, linux-kernel, devicetree
In-Reply-To: <2b882e9b-1d84-ea1f-edd5-d42aef701b5c@ti.com>
On 5/18/20 11:05 AM, Andrew F. Davis wrote:
> On 5/15/20 3:31 PM, Dan Murphy wrote:
>> From: Ricardo Rivera-Matos <r-rivera-matos@ti.com>
>>
>> Introduce the bq2515x family of chargers.
>>
>> The BQ2515X family of devices are highly integrated battery management
>> ICs that integrate the most common functions for wearbale devices
>
> s/wearbale/wearable
ACK
>
>
>> namely a charger, an output voltage rail, ADC for battery and system
>> monitoring, and a push-button controller.
>>
>> Datasheets:
>> bq25150 - http://www.ti.com/lit/ds/symlink/bq25150.pdf
>> bq25155 - http://www.ti.com/lit/ds/symlink/bq25155.pdf
>>
>> Signed-off-by: Ricardo Rivera-Matos <r-rivera-matos@ti.com>
>> ---
>> drivers/power/supply/Kconfig | 13 +
>> drivers/power/supply/Makefile | 1 +
>> drivers/power/supply/bq2515x_charger.c | 1176 ++++++++++++++++++++++++
>> 3 files changed, 1190 insertions(+)
>> create mode 100644 drivers/power/supply/bq2515x_charger.c
>>
>> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
>> index f3424fdce341..266193301e2d 100644
>> --- a/drivers/power/supply/Kconfig
>> +++ b/drivers/power/supply/Kconfig
>> @@ -589,6 +589,19 @@ config CHARGER_BQ24735
>> help
>> Say Y to enable support for the TI BQ24735 battery charger.
>>
>> +config CHARGER_BQ2515X
>> + tristate "TI BQ2515X battery charger family"
>> + depends on I2C
>> + depends on GPIOLIB || COMPILE_TEST
>> + select REGMAP_I2C
>> + help
>> + Say Y to enable support for the TI BQ2515X family of battery
>> + charging integrated circuits. The BQ2515X are highly integrated
>> + battery charge management ICs that integrate the most common
>> + functions for wearable devices, namely a charger, an output voltage
>> + rail, ADC for battery and system monitoring, and push-button
>> + controller.
>> +
>> config CHARGER_BQ25890
>> tristate "TI BQ25890 battery charger driver"
>> depends on I2C
>> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
>> index 6c7da920ea83..8fcc175a7e22 100644
>> --- a/drivers/power/supply/Makefile
>> +++ b/drivers/power/supply/Makefile
>> @@ -80,6 +80,7 @@ obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o
>> obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o
>> obj-$(CONFIG_CHARGER_BQ24257) += bq24257_charger.o
>> obj-$(CONFIG_CHARGER_BQ24735) += bq24735-charger.o
>> +obj-$(CONFIG_CHARGER_BQ2515X) += bq2515x_charger.o
>> obj-$(CONFIG_CHARGER_BQ25890) += bq25890_charger.o
>> obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o
>> obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o
>> diff --git a/drivers/power/supply/bq2515x_charger.c b/drivers/power/supply/bq2515x_charger.c
>> new file mode 100644
>> index 000000000000..78311168b171
>> --- /dev/null
>> +++ b/drivers/power/supply/bq2515x_charger.c
>> @@ -0,0 +1,1176 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// BQ2515X Battery Charger Driver
>> +// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
>> +
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>
> Is this header used? Looks like the chips have an INT pin for faults but
> this driver doesn't handle that yet.
ACK. Not yet, I will probably drop this until it becomes necessary or
desired.
>
>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>
> This one can also be dropped after addressing a comment below.
ACK
>
>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/power_supply.h>
>> +#include <linux/regmap.h>
>> +#include <linux/types.h>
>> +
>> +#define BQ2515X_MANUFACTURER "Texas Instruments"
>> +
>> +#define BQ2515X_STAT0 0x00
>> +#define BQ2515X_STAT1 0x01
>> +#define BQ2515X_STAT2 0x02
>> +#define BQ2515X_FLAG0 0x03
>> +#define BQ2515X_FLAG1 0x04
>> +#define BQ2515X_FLAG2 0x05
>> +#define BQ2515X_FLAG3 0x06
>> +#define BQ2515X_MASK0 0x07
>> +#define BQ2515X_MASK1 0x08
>> +#define BQ2515X_MASK2 0x09
>> +#define BQ2515X_MASK3 0x0a
>> +#define BQ2515X_VBAT_CTRL 0x12
>> +#define BQ2515X_ICHG_CTRL 0x13
>> +#define BQ2515X_PCHRGCTRL 0x14
>> +#define BQ2515X_TERMCTRL 0x15
>> +#define BQ2515X_BUVLO 0x16
>> +#define BQ2515X_CHARGERCTRL0 0x17
>> +#define BQ2515X_CHARGERCTRL1 0x18
>> +#define BQ2515X_ILIMCTRL 0x19
>> +#define BQ2515X_LDOCTRL 0x1d
>> +#define BQ2515X_MRCTRL 0x30
>> +#define BQ2515X_ICCTRL0 0x35
>> +#define BQ2515X_ICCTRL1 0x36
>> +#define BQ2515X_ICCTRL2 0x37
>> +#define BQ2515X_ADCCTRL0 0x40
>> +#define BQ2515X_ADCCTRL1 0x41
>> +#define BQ2515X_ADC_VBAT_M 0x42
>> +#define BQ2515X_ADC_VBAT_L 0x43
>> +#define BQ2515X_ADC_TS_M 0x44
>> +#define BQ2515X_ADC_TS_L 0x45
>> +#define BQ2515X_ADC_ICHG_M 0x46
>> +#define BQ2515X_ADC_ICHG_L 0x47
>> +#define BQ2515X_ADC_ADCIN_M 0x48
>> +#define BQ2515X_ADC_ADCIN_L 0x49
>> +#define BQ2515X_ADC_VIN_M 0x4a
>> +#define BQ2515X_ADC_VIN_L 0x4b
>> +#define BQ2515X_ADC_PMID_M 0x4c
>> +#define BQ2515X_ADC_PMID_L 0x4d
>> +#define BQ2515X_ADC_IIN_M 0x4e
>> +#define BQ2515X_ADC_IIN_L 0x4f
>> +#define BQ2515X_ADC_COMP1_M 0x52
>> +#define BQ2515X_ADC_COMP1_L 0X53
>> +#define BQ2515X_ADC_COMP2_M 0X54
>> +#define BQ2515X_ADC_COMP2_L 0x55
>> +#define BQ2515X_ADC_COMP3_M 0x56
>> +#define BQ2515X_ADC_COMP3_L 0x57
>> +#define BQ2515X_ADC_READ_EN 0x58
>> +#define BQ2515X_TS_FASTCHGCTRL 0x61
>> +#define BQ2515X_TS_COLD 0x62
>> +#define BQ2515X_TS_COOL 0x63
>> +#define BQ2515X_TS_WARM 0x64
>> +#define BQ2515X_TS_HOT 0x65
>> +#define BQ2515X_DEVICE_ID 0x6f
>> +
>> +#define BQ2515X_DEFAULT_ICHG_UA 10000
>> +#define BQ25150_DEFAULT_ILIM_UA 100000
>> +#define BQ25155_DEFAULT_ILIM_UA 500000
>> +#define BQ2515X_DEFAULT_VBAT_REG_UV 4200000
>> +#define BQ2515X_DEFAULT_IPRECHARGE_UA 2500
>> +
>> +#define BQ2515X_DIVISOR 65536
>> +#define BQ2515X_VBAT_BASE_VOLT 3600000
>> +#define BQ2515X_VBAT_REG_MAX 4600000
>> +#define BQ2515X_VBAT_REG_MIN 3600000
>> +#define BQ2515X_VBAT_STEP_UV 10000
>> +#define BQ2515X_UV_FACTOR 1000000
>> +#define BQ2515X_VBAT_MULTIPLIER 6
>> +#define BQ2515X_ICHG_DIVISOR 52429
>> +#define BQ2515X_ICHG_CURR_STEP_THRESH_UA 318750
>> +#define BQ2515X_ICHG_MIN_UA 0
>> +#define BQ2515X_ICHG_MAX_UA 500000
>> +#define BQ2515X_ICHG_RNG_1B0_UA 1250
>> +#define BQ2515X_ICHG_RNG_1B1_UA 2500
>> +#define BQ2515X_VLOWV_SEL_1B0_UV 3000000
>> +#define BQ2515X_VLOWV_SEL_1B1_UV 2800000
>> +#define BQ2515X_PRECHRG_ICHRG_RNGE_1875_UA 18750
>> +#define BQ2515X_PRECHRG_ICHRG_RNGE_3750_UA 37500
>> +#define BQ2515X_TWAKE2_MIN_US 1700000
>> +#define BQ2515X_TWAKE2_MAX_US 2300000
>> +
>> +#define BQ2515X_ILIM_150MA 0x2
>> +#define BQ2515X_ILIM_MASK 0x7
>> +#define BQ2515X_HEALTH_MASK 0xf
>> +#define BQ2515X_ICHGRNG_MASK 0x80
>> +#define BQ2515X_STAT0_MASK 0x0f
>> +#define BQ2515X_STAT1_MASK 0x1f
>> +#define BQ2515X_PRECHARGE_MASK 0x1f
>> +
>> +#define BQ2515X_TS_HOT_STAT BIT(0)
>> +#define BQ2515X_TS_WARM_STAT BIT(1)
>> +#define BQ2515X_TS_COOL_STAT BIT(2)
>> +#define BQ2515X_TS_COLD_STAT BIT(3)
>> +#define BQ2515X_SAFETY_TIMER_EXP BIT(5)
>> +
>> +#define BQ2515X_EN_VBAT_READ BIT(3)
>> +#define BQ2515X_EN_ICHG_READ BIT(5)
>> +
>> +#define BQ2515X_VIN_GOOD BIT(0)
>> +#define BQ2515X_CHRG_DONE BIT(5)
>> +#define BQ2515X_CV_CHRG_MODE BIT(6)
>> +
>> +#define BQ2515X_VIN_OVP_FAULT_STAT BIT(7)
>> +
>> +#define BQ2515X_WATCHDOG_DISABLE BIT(4)
>> +
>> +#define BQ2515X_ICHARGE_RANGE BIT(7)
>> +
>> +#define BQ2515X_VLOWV_SEL BIT(5)
>> +
>> +#define BQ2515X_CHARGER_DISABLE BIT(0)
>> +
>> +#define BQ2515X_HWRESET_14S_WD BIT(1)
>> +
>> +static const int bq2515x_ilim_lvl_values[] = {
>> + 50000, 100000, 150000, 200000, 300000, 400000, 500000, 600000
>> +};
>> +
>> +/**
>> + * struct bq2515x_init_data -
>> + * @ilim: input current limit
>> + */
>> +struct bq2515x_init_data {
>> + int ilim;
>> +};
>> +
>> +enum bq2515x_id {
>> + BQ25150,
>> + BQ25155,
>> +};
>> +
>> +/**
>> + * struct bq2515x_device -
>> + * @mains: mains properties
>> + * @battery: battery properties
>> + * @client: i2c client structure
>> + * @regmap: register map structure
>> + * @dev: device structure
>> + * @id: enumeration of device ids
>> + *
>> + * @reset_gpio: manual reset (MR) pin
>> + * @lp_gpio: low power mode pin
>> + * @ac_detect_gpio: power good (PG) pin
>> + * @ce_gpio: charge enable (CE) pin
>> + *
>> + * @model_name: string value describing device model
>> + * @device_id: value of device_id
>> + * @mains_online: boolean value indicating power supply online
>> + *
>> + * @bq2515x_init_data init_data: charger initialization data structure
>> + */
>> +struct bq2515x_device {
>> + struct power_supply *mains;
>> + struct power_supply *battery;
>> + struct i2c_client *client;
>
> 'client' doesn't seem used, it usually has use in the remove path, but
> this driver has everything devm managed, so not needed.
ACK
>
>
>> + struct regmap *regmap;
>> + struct device *dev;
>> + enum bq2515x_id id;
>> +
>> + struct gpio_desc *reset_gpio;
>> + struct gpio_desc *lp_gpio;
>> + struct gpio_desc *ac_detect_gpio;
>> + struct gpio_desc *ce_gpio;
>> +
>> + char model_name[I2C_NAME_SIZE];
>> + int device_id;
>
> This looks to be the same 'id' above, drop this one and use 'id'
> everywhere (or rename this ones type as enum bq2515x_id then remove the
> otherone).
ACK
>
>> + bool mains_online;
>> +
>> + struct bq2515x_init_data init_data;
>> +};
>> +
>> +static struct reg_default bq25150_reg_defs[] = {
>> + {BQ2515X_FLAG0, 0x0},
>> + {BQ2515X_FLAG1, 0x0},
>> + {BQ2515X_FLAG2, 0x0},
>> + {BQ2515X_FLAG3, 0x0},
>> + {BQ2515X_MASK0, 0x0},
>> + {BQ2515X_MASK1, 0x0},
>> + {BQ2515X_MASK2, 0x71},
>> + {BQ2515X_MASK3, 0x0},
>> + {BQ2515X_VBAT_CTRL, 0x3C},
>> + {BQ2515X_ICHG_CTRL, 0x8},
>> + {BQ2515X_PCHRGCTRL, 0x2},
>> + {BQ2515X_TERMCTRL, 0x14},
>> + {BQ2515X_BUVLO, 0x0},
>> + {BQ2515X_CHARGERCTRL0, 0x82},
>> + {BQ2515X_CHARGERCTRL1, 0x42},
>> + {BQ2515X_ILIMCTRL, 0x1},
>> + {BQ2515X_LDOCTRL, 0xB0},
>> + {BQ2515X_MRCTRL, 0x2A},
>> + {BQ2515X_ICCTRL0, 0x10},
>> + {BQ2515X_ICCTRL1, 0x0},
>> + {BQ2515X_ICCTRL2, 0x0},
>> + {BQ2515X_ADCCTRL0, 0x2},
>> + {BQ2515X_ADCCTRL1, 0x40},
>> + {BQ2515X_ADC_COMP1_M, 0x23},
>> + {BQ2515X_ADC_COMP1_L, 0x20},
>> + {BQ2515X_ADC_COMP2_M, 0x38},
>> + {BQ2515X_ADC_COMP2_L, 0x90},
>> + {BQ2515X_ADC_COMP3_M, 0x0},
>> + {BQ2515X_ADC_COMP3_L, 0x0},
>> + {BQ2515X_ADC_READ_EN, 0x0},
>> + {BQ2515X_TS_FASTCHGCTRL, 0x34},
>> + {BQ2515X_TS_COLD, 0x7C},
>> + {BQ2515X_TS_COOL, 0x6D},
>> + {BQ2515X_TS_WARM, 0x38},
>> + {BQ2515X_TS_HOT, 0x27},
>> + {BQ2515X_DEVICE_ID, 0x20},
>> +};
>> +
>> +static struct reg_default bq25155_reg_defs[] = {
>> + {BQ2515X_FLAG0, 0x0},
>> + {BQ2515X_FLAG1, 0x0},
>> + {BQ2515X_FLAG2, 0x0},
>> + {BQ2515X_FLAG3, 0x0},
>> + {BQ2515X_MASK0, 0x0},
>> + {BQ2515X_MASK1, 0x0},
>> + {BQ2515X_MASK2, 0x71},
>> + {BQ2515X_MASK3, 0x0},
>> + {BQ2515X_VBAT_CTRL, 0x3C},
>> + {BQ2515X_ICHG_CTRL, 0x8},
>> + {BQ2515X_PCHRGCTRL, 0x2},
>> + {BQ2515X_TERMCTRL, 0x14},
>> + {BQ2515X_BUVLO, 0x0},
>> + {BQ2515X_CHARGERCTRL0, 0x82},
>> + {BQ2515X_CHARGERCTRL1, 0xC2},
>> + {BQ2515X_ILIMCTRL, 0x6},
>> + {BQ2515X_LDOCTRL, 0xB0},
>> + {BQ2515X_MRCTRL, 0x2A},
>> + {BQ2515X_ICCTRL0, 0x10},
>> + {BQ2515X_ICCTRL1, 0x0},
>> + {BQ2515X_ICCTRL2, 0x40},
>> + {BQ2515X_ADCCTRL0, 0x2},
>> + {BQ2515X_ADCCTRL1, 0x40},
>> + {BQ2515X_ADC_COMP1_M, 0x23},
>> + {BQ2515X_ADC_COMP1_L, 0x20},
>> + {BQ2515X_ADC_COMP2_M, 0x38},
>> + {BQ2515X_ADC_COMP2_L, 0x90},
>> + {BQ2515X_ADC_COMP3_M, 0x0},
>> + {BQ2515X_ADC_COMP3_L, 0x0},
>> + {BQ2515X_ADC_READ_EN, 0x0},
>> + {BQ2515X_TS_FASTCHGCTRL, 0x34},
>> + {BQ2515X_TS_COLD, 0x7C},
>> + {BQ2515X_TS_COOL, 0x6D},
>> + {BQ2515X_TS_WARM, 0x38},
>> + {BQ2515X_TS_HOT, 0x27},
>> + {BQ2515X_DEVICE_ID, 0x35},
>> +};
>> +
>> +static bool bq2515x_is_ps_online(struct bq2515x_device *bq2515x)
>> +{
>> + return bq2515x->mains_online;
>
> Do we need a function for this? Just use 'bq2515x->mains_online' directly.
ACK
>
>
>> +}
>> +
>> +static int bq2515x_wake_up(struct bq2515x_device *bq2515x)
>> +{
>> + int ret;
>> + int val;
>> +
>> + /* Read the STAT register if we can read it then the device is out
>> + * of ship mode. If the register cannot be read then attempt to wake
>> + * it up and enable the ADC.
>> + */
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &val);
>> + if (ret)
>> + return ret;
>> +
>> + /* Need to toggle LP and bring device out of ship mode. The device
>> + * will exit the ship mode when the MR pin is held low for at least
>> + * t_WAKE2 as shown in section 8.3.7.1 of the datasheet.
>> + */
>> + gpiod_set_value_cansleep(bq2515x->lp_gpio, 0);
>> +
>> + gpiod_set_value_cansleep(bq2515x->reset_gpio, 0);
>> + usleep_range(BQ2515X_TWAKE2_MIN_US, BQ2515X_TWAKE2_MAX_US);
>> + gpiod_set_value_cansleep(bq2515x->reset_gpio, 1);
>> +
>> + return regmap_write(bq2515x->regmap, BQ2515X_ADC_READ_EN,
>> + (BQ2515X_EN_VBAT_READ | BQ2515X_EN_ICHG_READ));
>> +}
>> +
>> +static int bq2515x_update_ps_status(struct bq2515x_device *bq2515x)
>> +{
>> + bool dc = false;
>> + unsigned int val;
>> + int ret;
>> +
>> + if (bq2515x->ac_detect_gpio)
>> + val = gpiod_get_value_cansleep(bq2515x->ac_detect_gpio);
>> + else {
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &val);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + dc = val & BQ2515X_VIN_GOOD;
>> +
>> + ret = bq2515x->mains_online != dc;
>> +
>> + bq2515x->mains_online = dc;
>> +
>> + return ret;
>> +}
>> +
>> +static int bq2515x_disable_watchdog_timers(struct bq2515x_device *bq2515x)
>> +{
>> + int ret;
>> +
>> + ret = regmap_update_bits(bq2515x->regmap, BQ2515X_CHARGERCTRL0,
>> + BQ2515X_WATCHDOG_DISABLE, BQ2515X_WATCHDOG_DISABLE);
>> + if (ret)
>> + return ret;
>> +
>> + return regmap_update_bits(bq2515x->regmap, BQ2515X_ICCTRL2,
>> + BQ2515X_HWRESET_14S_WD, 0);
>> +}
>> +
>> +static int bq2515x_get_battery_voltage_now(struct bq2515x_device *bq2515x)
>> +{
>> + int ret;
>> + int vbat_msb;
>> + int vbat_lsb;
>> + uint32_t vbat_measurement;
>> +
>> + if (!bq2515x_is_ps_online(bq2515x))
>> + bq2515x_wake_up(bq2515x);
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_VBAT_M, &vbat_msb);
>> + if (ret)
>> + return ret;
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_VBAT_L, &vbat_lsb);
>> + if (ret)
>> + return ret;
>> +
>> + vbat_measurement = (vbat_msb << 8) | vbat_lsb;
>> +
>> + return vbat_measurement * (BQ2515X_UV_FACTOR / BQ2515X_DIVISOR) *
>> + BQ2515X_VBAT_MULTIPLIER;
>> +}
>> +
>> +static int bq2515x_get_battery_current_now(struct bq2515x_device *bq2515x)
>> +{
>> + int ret;
>> + int ichg_msb;
>> + int ichg_lsb;
>> + uint32_t ichg_measurement;
>> + u16 ichg_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
>> + unsigned int ichg_reg_code, reg_code;
>> + unsigned int icharge_range = 0, pchrgctrl;
>> + unsigned int buvlo, vlowv_sel, vlowv = BQ2515X_VLOWV_SEL_1B0_UV;
>> +
>> + if (!bq2515x_is_ps_online(bq2515x))
>> + return -ENODATA;
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_ICHG_M, &ichg_msb);
>> + if (ret)
>> + return ret;
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_ICHG_L, &ichg_lsb);
>> + if (ret)
>> + return ret;
>> +
>> + ichg_measurement = (ichg_msb << 8) | ichg_lsb;
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_BUVLO, &buvlo);
>> + if (ret)
>> + return ret;
>> +
>> + vlowv_sel = buvlo & BQ2515X_VLOWV_SEL;
>> +
>> + if (vlowv_sel)
>> + vlowv = BQ2515X_VLOWV_SEL_1B1_UV;
>> +
>> + if (bq2515x_get_battery_voltage_now(bq2515x) < vlowv) {
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL,
>> + &pchrgctrl);
>> + if (ret)
>> + return ret;
>> +
>> + reg_code = pchrgctrl & BQ2515X_PRECHARGE_MASK;
>> + } else {
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_ICHG_CTRL,
>> + &ichg_reg_code);
>> + if (ret)
>> + return ret;
>> +
>> + reg_code = ichg_reg_code;
>> + }
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL, &pchrgctrl);
>> + if (ret)
>> + return ret;
>> +
>> + icharge_range = pchrgctrl & BQ2515X_ICHARGE_RANGE;
>> +
>> + if (icharge_range)
>> + ichg_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
>> +
>> + return reg_code * (ichg_multiplier * ichg_measurement /
>> + BQ2515X_ICHG_DIVISOR);
>> +}
>> +
>> +static bool bq2515x_get_charge_disable(struct bq2515x_device *bq2515x)
>> +{
>> + int ret;
>> + int ce_pin;
>> + int icctrl2;
>> + int charger_disable;
>> +
>> + ce_pin = gpiod_get_value_cansleep(bq2515x->ce_gpio);
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_ICCTRL2, &icctrl2);
>> + if (ret)
>> + return ret;
>> +
>> + charger_disable = icctrl2 & BQ2515X_CHARGER_DISABLE;
>> +
>> + if (charger_disable || ce_pin)
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +static int bq2515x_set_charge_disable(struct bq2515x_device *bq2515x, int val)
>> +{
>> + gpiod_set_value_cansleep(bq2515x->ce_gpio, val);
>> +
>> + return regmap_update_bits(bq2515x->regmap, BQ2515X_ICCTRL2,
>> + BQ2515X_CHARGER_DISABLE, val);
>> +}
>> +
>> +static int bq2515x_get_const_charge_current(struct bq2515x_device *bq2515x)
>> +{
>> + int ret;
>> + u16 ichg_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
>> + unsigned int ichg_reg_code;
>> + unsigned int pchrgctrl;
>> + unsigned int icharge_range;
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_ICHG_CTRL, &ichg_reg_code);
>> + if (ret)
>> + return ret;
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL, &pchrgctrl);
>> + if (ret)
>> + return ret;
>> +
>> + icharge_range = pchrgctrl & BQ2515X_ICHARGE_RANGE;
>> +
>> + if (icharge_range)
>> + ichg_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
>> +
>> + return ichg_reg_code * ichg_multiplier;
>> +}
>> +
>> +static int bq2515x_set_const_charge_current(struct bq2515x_device *bq2515x,
>> + int val)
>> +{
>> + int ret;
>> + unsigned int ichg_reg_code;
>> + u16 ichg_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
>> + unsigned int icharge_range = 0;
>> +
>> + if (val > BQ2515X_ICHG_MAX_UA || val < BQ2515X_ICHG_MIN_UA)
>> + return -EINVAL;
>> +
>> + if (val > BQ2515X_ICHG_CURR_STEP_THRESH_UA) {
>> + ichg_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
>> + icharge_range = BQ2515X_ICHARGE_RANGE;
>> + }
>> +
>> + bq2515x_set_charge_disable(bq2515x, 1);
>> +
>> + ret = regmap_update_bits(bq2515x->regmap, BQ2515X_PCHRGCTRL,
>> + BQ2515X_ICHARGE_RANGE, icharge_range);
>> + if (ret)
>> + return ret;
>> +
>> + ichg_reg_code = val / ichg_multiplier;
>> +
>> + ret = regmap_write(bq2515x->regmap, BQ2515X_ICHG_CTRL, ichg_reg_code);
>> + if (ret)
>> + return ret;
>> +
>> + return bq2515x_set_charge_disable(bq2515x, 0);
>> +}
>> +
>> +static int bq2515x_get_precharge_current(struct bq2515x_device *bq2515x)
>> +{
>> + int ret;
>> + unsigned int pchrgctrl;
>> + unsigned int icharge_range;
>> + u16 precharge_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
>> + unsigned int precharge_reg_code;
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL, &pchrgctrl);
>> + if (ret)
>> + return ret;
>> +
>> + icharge_range = pchrgctrl & BQ2515X_ICHARGE_RANGE;
>> +
>> + if (icharge_range)
>> + precharge_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
>> +
>> + precharge_reg_code = pchrgctrl & BQ2515X_PRECHARGE_MASK;
>> +
>> + return precharge_reg_code * precharge_multiplier;
>> +}
>> +
>> +static int bq2515x_set_precharge_current(struct bq2515x_device *bq2515x,
>> + int val)
>> +{
>> + int ret;
>> + unsigned int pchrgctrl;
>> + unsigned int icharge_range;
>> + unsigned int precharge_reg_code;
>> + u16 precharge_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
>> + u16 precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_1875_UA;
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL, &pchrgctrl);
>> + if (ret)
>> + return ret;
>> +
>> + icharge_range = pchrgctrl & BQ2515X_ICHARGE_RANGE;
>> +
>> + if (icharge_range) {
>> + precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_3750_UA;
>> + precharge_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
>> + }
>> + if (val > precharge_max_ua || val < BQ2515X_ICHG_MIN_UA)
>> + return -EINVAL;
>> +
>> + precharge_reg_code = val / precharge_multiplier;
>> +
>> + ret = bq2515x_set_charge_disable(bq2515x, 1);
>> + if (ret)
>> + return ret;
>> +
>> + ret = regmap_update_bits(bq2515x->regmap, BQ2515X_PCHRGCTRL,
>> + BQ2515X_PRECHARGE_MASK, precharge_reg_code);
>> + if (ret)
>> + return ret;
>> +
>> + return bq2515x_set_charge_disable(bq2515x, 0);
>> +}
>> +
>> +static int bq2515x_charging_status(struct bq2515x_device *bq2515x,
>> + union power_supply_propval *val)
>> +{
>> + bool status0_no_fault;
>> + bool status1_no_fault;
>> + bool ce_status;
>> + bool charge_done;
>> + unsigned int status;
>> + int ret;
>> +
>> + if (!bq2515x_is_ps_online(bq2515x)) {
>> + val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
>> + return 0;
>> + }
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &status);
>> + if (ret)
>> + return ret;
>> +
>> + /*
>> + * The code block below is used to determine if any faults from the
>> + * STAT0 register are disbaling charging or if the charge has completed
>> + * according to the CHARGE_DONE_STAT bit.
>> + */
>> + if (((status & BQ2515X_STAT0_MASK) == true) &
>> + ((status & BQ2515X_CHRG_DONE) == false)) {
>> + status0_no_fault = true;
>> + charge_done = false;
>> + } else if (status & BQ2515X_CHRG_DONE) {
>> + charge_done = true;
>> + status0_no_fault = false;
>> + } else {
>> + status0_no_fault = false;
>> + charge_done = false;
>> + }
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT1, &status);
>> + if (ret)
>> + return ret;
>> + /*
>> + * The code block below is used to determine if any faults from the
>> + * STAT1 register are disbaling charging
>> + */
>> + if ((status & BQ2515X_STAT1_MASK) == false)
>> + status1_no_fault = true;
>> + else
>> + status1_no_fault = false;
>> +
>> + ce_status = (!bq2515x_get_charge_disable(bq2515x));
>> +
>> + /*
>> + * If there are no faults and charging is enabled, then status is
>> + * charging. Otherwise, if charging is complete, then status is full.
>> + * Otherwise, if a fault exists or charging is disabled, then status is
>> + * not charging
>> + */
>> + if (status0_no_fault & status1_no_fault & ce_status)
>> + val->intval = POWER_SUPPLY_STATUS_CHARGING;
>> + else if (charge_done)
>> + val->intval = POWER_SUPPLY_STATUS_FULL;
>> + else if (!(status0_no_fault & status1_no_fault & ce_status))
>> + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
>> +
>> + return 0;
>> +}
>> +
>> +static int bq2515x_get_batt_reg(struct bq2515x_device *bq2515x)
>> +{
>> + int vbat_reg_code;
>> + int ret;
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_VBAT_CTRL, &vbat_reg_code);
>> + if (ret)
>> + return ret;
>> +
>> + return BQ2515X_VBAT_BASE_VOLT + vbat_reg_code * BQ2515X_VBAT_STEP_UV;
>> +}
>> +
>> +static int bq2515x_set_batt_reg(struct bq2515x_device *bq2515x, int val)
>> +{
>> + int vbat_reg_code;
>> +
>> + if (val > BQ2515X_VBAT_REG_MAX || val < BQ2515X_VBAT_REG_MIN)
>> + return -EINVAL;
>> +
>> + vbat_reg_code = (val - BQ2515X_VBAT_BASE_VOLT) / BQ2515X_VBAT_STEP_UV;
>> +
>> + return regmap_write(bq2515x->regmap, BQ2515X_VBAT_CTRL, vbat_reg_code);
>> +}
>> +
>> +static int bq2515x_get_ilim_lvl(struct bq2515x_device *bq2515x)
>> +{
>> + int ret;
>> + int ilimctrl;
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_ILIMCTRL, &ilimctrl);
>> + if (ret)
>> + return ret;
>> +
>> + return bq2515x_ilim_lvl_values[ilimctrl & BQ2515X_ILIM_MASK];
>> +}
>> +
>> +static int bq2515x_set_ilim_lvl(struct bq2515x_device *bq2515x, int val)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(bq2515x_ilim_lvl_values); i++) {
>> + if (val == bq2515x_ilim_lvl_values[i])
>> + break;
>> +
>> + if (val > bq2515x_ilim_lvl_values[i - 1] &&
>> + val < bq2515x_ilim_lvl_values[i]) {
>> + if (val - bq2515x_ilim_lvl_values[i - 1] <
>> + bq2515x_ilim_lvl_values[i] - val) {
>> + i = i - 1;
>> + break;
>
> This is a little messy, and it's not immediately obvious it will always
> work, in fact thinking about this, what happends in the else case.. the
> value is closer to the [i] value, then we just never find it (I don't
> think that is what we want, to only find the closest value below the next.)
>
> Lucky for us, there are helper macros already in kernel to do most of
> these tasks (I belive there is one for finding the closest value in an
> array, but I'll leave finding it for you :) (if it exists..)).
There seems to be one for matching a given string in an array called
match_string, but nothing for matching an integer. I will add
conditional statement to prevent an invalid input and avoid the else case.
>
>
>> + }
>> + }
>> + }
>> +
>> + return regmap_write(bq2515x->regmap, BQ2515X_ILIMCTRL, i);
>> +}
>> +
>> +static int bq2515x_power_supply_property_is_writeable(struct power_supply *psy,
>> + enum power_supply_property prop)
>> +{
>> + switch (prop) {
>> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
>> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
>> + case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
>> + return true;
>> + default:
>> + return false;
>> + }
>> +}
>> +
>> +static int bq2515x_charger_get_health(struct bq2515x_device *bq2515x,
>> + union power_supply_propval *val)
>> +{
>> + int health = POWER_SUPPLY_HEALTH_GOOD;
>> + int ret;
>> + unsigned int stat1;
>> + unsigned int flag3;
>> +
>> + if (!bq2515x_is_ps_online(bq2515x))
>> + bq2515x_wake_up(bq2515x);
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_FLAG3, &flag3);
>> + if (ret)
>> + return ret;
>> +
>> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT1, &stat1);
>> + if (ret)
>> + return ret;
>> +
>> + if (stat1 & BQ2515X_HEALTH_MASK) {
>> + switch (stat1 & BQ2515X_HEALTH_MASK) {
>> + case BQ2515X_TS_HOT_STAT:
>> + health = POWER_SUPPLY_HEALTH_HOT;
>> + break;
>> + case BQ2515X_TS_WARM_STAT:
>> + health = POWER_SUPPLY_HEALTH_WARM;
>> + break;
>> + case BQ2515X_TS_COOL_STAT:
>> + health = POWER_SUPPLY_HEALTH_COOL;
>> + break;
>> + case BQ2515X_TS_COLD_STAT:
>> + health = POWER_SUPPLY_HEALTH_COLD;
>> + break;
>> + default:
>> + health = POWER_SUPPLY_HEALTH_UNKNOWN;
>> + break;
>> + }
>> + }
>> +
>> + if (stat1 & BQ2515X_VIN_OVP_FAULT_STAT)
>> + health = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
>> +
>> + if (flag3 & BQ2515X_SAFETY_TIMER_EXP)
>> + health = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
>> +
>> + val->intval = health;
>> + return 0;
>> +}
>> +
>> +static int bq2515x_mains_set_property(struct power_supply *psy,
>> + enum power_supply_property prop,
>> + const union power_supply_propval *val)
>> +{
>> + struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
>> + int ret;
>> +
>> + switch (prop) {
>> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>> + ret = bq2515x_set_ilim_lvl(bq2515x, val->intval);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int bq2515x_battery_set_property(struct power_supply *psy,
>> + enum power_supply_property prop,
>> + const union power_supply_propval *val)
>> +{
>> + struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
>> + int ret;
>> +
>> + switch (prop) {
>> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
>> + ret = bq2515x_set_batt_reg(bq2515x, val->intval);
>> + break;
>> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
>> + ret = bq2515x_set_const_charge_current(bq2515x, val->intval);
>> + break;
>> + case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
>> + ret = bq2515x_set_precharge_current(bq2515x, val->intval);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int bq2515x_mains_get_property(struct power_supply *psy,
>> + enum power_supply_property prop,
>> + union power_supply_propval *val)
>> +{
>> + struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
>> + int ret = 0;
>> +
>> + switch (prop) {
>> + case POWER_SUPPLY_PROP_ONLINE:
>> + val->intval = bq2515x->mains_online;
>> + break;
>> + case POWER_SUPPLY_PROP_HEALTH:
>> + ret = bq2515x_charger_get_health(bq2515x, val);
>> + if (ret)
>> + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
>> + break;
>> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
>> + ret = bq2515x_get_ilim_lvl(bq2515x);
>> + if (ret)
>> + return ret;
>> +
>> + val->intval = ret;
>> + break;
>> + case POWER_SUPPLY_PROP_MODEL_NAME:
>> + val->strval = bq2515x->model_name;
>> + break;
>> + case POWER_SUPPLY_PROP_MANUFACTURER:
>> + val->strval = BQ2515X_MANUFACTURER;
>> + break;
>> + case POWER_SUPPLY_PROP_STATUS:
>> + ret = bq2515x_charging_status(bq2515x, val);
>> + if (ret)
>> + return ret;
>> + break;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int bq2515x_battery_get_property(struct power_supply *psy,
>> + enum power_supply_property prop,
>> + union power_supply_propval *val)
>> +{
>> + struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
>> + int ret;
>> +
>> + ret = bq2515x_update_ps_status(bq2515x);
>> + if (ret)
>> + return ret;
>> +
>> + switch (prop) {
>> +
>> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>> + ret = bq2515x_get_battery_voltage_now(bq2515x);
>> + if (ret < 0)
>> + return ret;
>> +
>> + val->intval = ret;
>> + break;
>> + case POWER_SUPPLY_PROP_CURRENT_NOW:
>> + ret = bq2515x_get_battery_current_now(bq2515x);
>> + if (ret < 0)
>> + return ret;
>> +
>> + val->intval = ret;
>> + break;
>> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
>> + ret = bq2515x_get_const_charge_current(bq2515x);
>> + if (ret < 0)
>> + return ret;
>> +
>> + val->intval = ret;
>> + break;
>> + case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
>> + ret = bq2515x_get_precharge_current(bq2515x);
>> + if (ret < 0)
>> + return ret;
>> +
>> + val->intval = ret;
>> + break;
>> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
>> + ret = bq2515x_get_batt_reg(bq2515x);
>> + if (ret < 0)
>> + return ret;
>> +
>> + val->intval = ret;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + return 0;
>> +}
>> +
>> +static enum power_supply_property bq2515x_battery_properties[] = {
>> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
>> + POWER_SUPPLY_PROP_CURRENT_NOW,
>> + POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
>> + POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
>> + POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
>> +};
>> +
>> +static enum power_supply_property bq2515x_charger_properties[] = {
>> + POWER_SUPPLY_PROP_ONLINE,
>> + POWER_SUPPLY_PROP_STATUS,
>> + POWER_SUPPLY_PROP_HEALTH,
>> + POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
>> + POWER_SUPPLY_PROP_MODEL_NAME,
>> + POWER_SUPPLY_PROP_MANUFACTURER,
>> +};
>> +
>> +static struct power_supply_desc bq2515x_mains_desc = {
>> + .name = "bq2515x-mains",
>> + .type = POWER_SUPPLY_TYPE_MAINS,
>> + .get_property = bq2515x_mains_get_property,
>> + .set_property = bq2515x_mains_set_property,
>> + .properties = bq2515x_charger_properties,
>> + .num_properties = ARRAY_SIZE(bq2515x_charger_properties),
>> + .property_is_writeable = bq2515x_power_supply_property_is_writeable,
>> +};
>> +
>> +static struct power_supply_desc bq2515x_battery_desc = {
>> + .name = "bq2515x-battery",
>> + .type = POWER_SUPPLY_TYPE_BATTERY,
>> + .get_property = bq2515x_battery_get_property,
>> + .set_property = bq2515x_battery_set_property,
>> + .properties = bq2515x_battery_properties,
>> + .num_properties = ARRAY_SIZE(bq2515x_battery_properties),
>> + .property_is_writeable = bq2515x_power_supply_property_is_writeable,
>> +};
>> +
>> +static int bq2515x_power_supply_register(struct bq2515x_device *bq2515x,
>> + struct device *dev)
>> +{
>> + struct power_supply_config psy_cfg = { .drv_data = bq2515x,
>> + .of_node = dev->of_node, };
>> +
>> + bq2515x->mains = devm_power_supply_register(bq2515x->dev,
>> + &bq2515x_mains_desc,
>> + &psy_cfg);
>> + if (IS_ERR(bq2515x->mains))
>> + return -EINVAL;
>> +
>> + bq2515x->battery = devm_power_supply_register(bq2515x->dev,
>> + &bq2515x_battery_desc,
>> + &psy_cfg);
>> + if (IS_ERR(bq2515x->battery))
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>> +static int bq2515x_hw_init(struct bq2515x_device *bq2515x)
>> +{
>> + int ret;
>> + struct power_supply_battery_info bat_info = { };
>> +
>> + ret = bq2515x_disable_watchdog_timers(bq2515x);
>> + if (ret)
>> + return ret;
>> +
>> + if (bq2515x->init_data.ilim) {
>> + ret = bq2515x_set_ilim_lvl(bq2515x, bq2515x->init_data.ilim);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + ret = power_supply_get_battery_info(bq2515x->mains, &bat_info);
>> + if (ret) {
>> + dev_warn(bq2515x->dev, "battery info missing, default values will be applied\n");
>> +
>> + bat_info.constant_charge_current_max_ua =
>> + BQ2515X_DEFAULT_ICHG_UA;
>> +
>> + bat_info.constant_charge_voltage_max_uv =
>> + BQ2515X_DEFAULT_VBAT_REG_UV;
>> +
>> + bat_info.precharge_current_ua =
>> + BQ2515X_DEFAULT_IPRECHARGE_UA;
>> + }
>> +
>> + ret = bq2515x_set_const_charge_current(bq2515x,
>> + bat_info.constant_charge_current_max_ua);
>> + if (ret)
>> + return ret;
>> +
>> + ret = bq2515x_set_batt_reg(bq2515x,
>> + bat_info.constant_charge_voltage_max_uv);
>> + if (ret)
>> + return ret;
>> +
>> + return bq2515x_set_precharge_current(bq2515x,
>> + bat_info.precharge_current_ua);
>> +}
>> +
>> +static int bq2515x_read_properties(struct bq2515x_device *bq2515x)
>> +{
>> + int ret;
>> +
>> + ret = device_property_read_u32(bq2515x->dev,
>> + "input-current-limit-microamp",
>> + &bq2515x->init_data.ilim);
>> + if (ret)
>> + switch (bq2515x->device_id) {
>> + case BQ25150:
>> + bq2515x->init_data.ilim = BQ25150_DEFAULT_ILIM_UA;
>> + break;
>> + case BQ25155:
>> + bq2515x->init_data.ilim = BQ25155_DEFAULT_ILIM_UA;
>> + break;
>> + }
>> +
>> + bq2515x->ac_detect_gpio = devm_gpiod_get_optional(bq2515x->dev,
>> + "ac-detect", GPIOD_IN);
>> + if (IS_ERR(bq2515x->ac_detect_gpio)) {
>> + ret = PTR_ERR(bq2515x->ac_detect_gpio);
>> + if (ret != -ENODEV) {
>> + dev_err(bq2515x->dev, "Failed to get ac detect: %d\n",
>> + ret);
>
> Do we need to print 'ret' here? It is print out in the caller on
> returning, plus the alignment wrap is really not so pretty here..
ACK
>> + return ret;
>> + }
>> + bq2515x->ac_detect_gpio = NULL;
>> + }
>> +
>> + bq2515x->reset_gpio = devm_gpiod_get_optional(bq2515x->dev,
>> + "reset", GPIOD_OUT_LOW);
>> + if (IS_ERR(bq2515x->reset_gpio)) {
>> + ret = PTR_ERR(bq2515x->reset_gpio);
>> + if (ret != -ENODEV) {
>> + dev_err(bq2515x->dev, "Failed to get reset: %d\n",
>> + ret);
>> + return ret;
>> + }
>> + bq2515x->reset_gpio = NULL;
>> + }
>> +
>> + bq2515x->lp_gpio = devm_gpiod_get_optional(bq2515x->dev, "low-power",
>> + GPIOD_OUT_LOW);
>> + if (IS_ERR(bq2515x->lp_gpio)) {
>> + ret = PTR_ERR(bq2515x->lp_gpio);
>> + if (ret != -ENODEV) {
>> + dev_err(bq2515x->dev, "Failed to get low power: %d\n",
>> + ret);
>> + return ret;
>> + }
>> + bq2515x->lp_gpio = NULL;
>> + }
>> +
>> + bq2515x->ce_gpio = devm_gpiod_get_optional(bq2515x->dev,
>> + "charge-enable",
>> + GPIOD_OUT_LOW);
>> + if (IS_ERR(bq2515x->ce_gpio)) {
>> + ret = PTR_ERR(bq2515x->ce_gpio);
>> + if (ret != -ENODEV) {
>> + dev_err(bq2515x->dev, "Failed to get ce %d\n", ret);
>> + return ret;
>> + }
>> + bq2515x->ce_gpio = NULL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static bool bq2515x_volatile_register(struct device *dev, unsigned int reg)
>> +{
>> + switch (reg) {
>> + case BQ2515X_STAT0 ... BQ2515X_FLAG3:
>> + case BQ2515X_ADC_VBAT_M ... BQ2515X_ADC_IIN_L:
>> + return true;
>> + default:
>> + return false;
>> + }
>> +}
>> +
>> +static const struct regmap_config bq25150_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> +
>> + .max_register = BQ2515X_DEVICE_ID,
>> + .reg_defaults = bq25150_reg_defs,
>
> NITPICK: 'defs' reads to me like definitions, maybe just use _defautls.
ACK
>
>
>> + .num_reg_defaults = ARRAY_SIZE(bq25150_reg_defs),
>> + .cache_type = REGCACHE_RBTREE,
>> + .volatile_reg = bq2515x_volatile_register,
>> +};
>> +
>> +static const struct regmap_config bq25155_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> +
>> + .max_register = BQ2515X_DEVICE_ID,
>> + .reg_defaults = bq25155_reg_defs,
>> + .num_reg_defaults = ARRAY_SIZE(bq25155_reg_defs),
>> + .cache_type = REGCACHE_RBTREE,
>> + .volatile_reg = bq2515x_volatile_register,
>> +};
>> +
>> +static int bq2515x_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct device *dev = &client->dev;
>> + struct bq2515x_device *bq;
>> + int ret;
>> +
>> + bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
>> + if (!bq)
>> + return -ENOMEM;
>> +
>> + bq->client = client;
>> + bq->dev = dev;
>> +
>> + strncpy(bq->model_name, id->name, I2C_NAME_SIZE);
>> +
>> + bq->device_id = (enum bq2515x_id)of_device_get_match_data(&client->dev);
>
> This grabs the 'data' field of of_device_id table below which will
> always be zero as you do not define the IDs there. You are actually
> wanting to grab from 'data' in i2c_device_id table. So replace this line
> with:
>
> bq->device_id = id->driver_data;
>
> After that since you don't use any of_* functions, drop the
> 'of_device.h' header include. (Idea here is that most drivers should not
> use of_ functions directly anyway).
ACK
>
>
>> +
>> + switch (bq->device_id) {
>> + case BQ25150:
>> + bq->regmap = devm_regmap_init_i2c(client,
>> + &bq25150_regmap_config);
>> + break;
>> + case BQ25155:
>> + bq->regmap = devm_regmap_init_i2c(client,
>> + &bq25155_regmap_config);
>> + break;
>> + }
>> +
>> + if (IS_ERR(bq->regmap)) {
>> + dev_err(dev, "failed to allocate register map\n");
>> + return PTR_ERR(bq->regmap);
>> + }
>> +
>> + i2c_set_clientdata(client, bq);
>> +
>> + ret = bq2515x_read_properties(bq);
>> + if (ret) {
>> + dev_err(dev, "Failed to read device tree properties %d\n",
>> + ret);
>> + return ret;
>> + }
>> +
>> + ret = bq2515x_power_supply_register(bq, dev);
>> + if (ret) {
>> + dev_err(dev, "failed to register power supply\n");
>> + return ret;
>> + }
>> +
>
> Is this order right here? Usually the last thing you do is register with
> the framework, doing it before all the device init is done could lead to
> some accessor functions in this driver getting called before the below
> fully inits the chip.
You cannot use power_supply_get_battery_info in hw_init until you have
registered the power supply. Otherwise you get a kernel oops. This can
be seen in the bq24190 driver.
>
> Andrew
>
>
>> + ret = bq2515x_hw_init(bq);
>> + if (ret) {
>> + dev_err(dev, "Cannot initialize the chip\n");
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct i2c_device_id bq2515x_i2c_ids[] = {
>> + { "bq25150", BQ25150, },
>> + { "bq25155", BQ25155, },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(i2c, bq2515x_i2c_ids);
>> +
>> +static const struct of_device_id bq2515x_of_match[] = {
>> + { .compatible = "ti,bq25150", },
>> + { .compatible = "ti,bq25155", },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, bq2515x_of_match);
>> +
>> +static struct i2c_driver bq2515x_driver = {
>> + .driver = {
>> + .name = "bq2515x-charger",
>> + .of_match_table = bq2515x_of_match,
>> + },
>> + .probe = bq2515x_probe,
>> + .id_table = bq2515x_i2c_ids,
>> +};
>> +module_i2c_driver(bq2515x_driver);
>> +
>> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
>> +MODULE_AUTHOR("Ricardo Rivera-Matos <r-rivera-matos@ti.com>");
>> +MODULE_DESCRIPTION("BQ2515X charger driver");
>> +MODULE_LICENSE("GPL v2");
>>
^ permalink raw reply
* Re: [PATCH V1 2/3] mmc: sdhci-msm: Use internal voltage control
From: Bjorn Andersson @ 2020-05-18 19:57 UTC (permalink / raw)
To: Veerabhadrarao Badiganti
Cc: adrian.hunter, ulf.hansson, robh+dt, linux-mmc, linux-kernel,
linux-arm-msm, devicetree, Vijay Viswanath, Asutosh Das,
Andy Gross
In-Reply-To: <1589541535-8523-3-git-send-email-vbadigan@codeaurora.org>
On Fri 15 May 04:18 PDT 2020, Veerabhadrarao Badiganti wrote:
> From: Vijay Viswanath <vviswana@codeaurora.org>
>
> On qcom SD host controllers voltage switching be done after the HW
> is ready for it. The HW informs its readiness through power irq.
> The voltage switching should happen only then.
>
> Use the internal voltage switching and then control the voltage
> switching using power irq.
>
> Set the regulator load as well so that regulator can be configured
> in LPM mode when in is not being used.
>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> Signed-off-by: Vijay Viswanath <vviswana@codeaurora.org>
> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
Please note that per Documentation/process/submitting-patches.rst
section 11) this states:
1) You wrote the patch (From:) without stating that its Certificate of
origin.
2) Then Asutosh took your patch (in full or part) and guarantees that
he's allowed to contribute it to the project.
3) Then you took his patch (in full or part) and guarantee that you're
allowed to contribute it to the project.
4) Then Veerabhadrarao took your patch (in full or part) and guarantees
that he's allowed to contribute it to the project
5) Then somehow it came out of your inbox - even if Veerabhadrarao was
the one who handled it last.
As author you should be the first one to certify, and as poster to LKML
you should be the last one.
If you worked together on this, then list Asutosh and Veerabhadrarao as
Co-developed-by.
> ---
> drivers/mmc/host/sdhci-msm.c | 215 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 207 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 97758fa..a10e955 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -36,7 +36,9 @@
> #define CORE_PWRCTL_IO_LOW BIT(2)
> #define CORE_PWRCTL_IO_HIGH BIT(3)
> #define CORE_PWRCTL_BUS_SUCCESS BIT(0)
> +#define CORE_PWRCTL_BUS_FAIL BIT(1)
> #define CORE_PWRCTL_IO_SUCCESS BIT(2)
> +#define CORE_PWRCTL_IO_FAIL BIT(3)
> #define REQ_BUS_OFF BIT(0)
> #define REQ_BUS_ON BIT(1)
> #define REQ_IO_LOW BIT(2)
> @@ -263,6 +265,9 @@ struct sdhci_msm_host {
> bool use_cdr;
> u32 transfer_mode;
> bool updated_ddr_cfg;
> + u32 vmmc_load;
> + u32 vqmmc_load;
> + bool vqmmc_enabled;
> };
>
> static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
> @@ -1298,6 +1303,78 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
> sdhci_msm_hs400(host, &mmc->ios);
> }
>
> +static int sdhci_msm_set_vmmc(struct sdhci_msm_host *msm_host,
> + struct mmc_host *mmc, int level)
> +{
> + int load, ret;
> +
> + if (IS_ERR(mmc->supply.vmmc))
> + return 0;
> +
> + if (msm_host->vmmc_load) {
> + load = level ? msm_host->vmmc_load : 0;
> + ret = regulator_set_load(mmc->supply.vmmc, load);
I started on the comment about regulator_set_load() that you can find
below...
> + if (ret)
> + goto out;
> + }
> +
> + ret = mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, mmc->ios.vdd);
...but I don't see that mmc->ios.vdd necessarily is in sync with
"level". Or do you here simply set the load based on what the hardware
tell you and then orthogonally to that let the core enable/disable the
regulator?
Perhaps I'm just missing something obvious, but if not I believe this
warrants a comment describing that you're lowering the power level
regardless of the actual power being disabled.
> +out:
> + if (ret)
> + pr_err("%s: vmmc set load/ocr failed: %d\n",
> + mmc_hostname(mmc), ret);
Please use:
dev_err(mmc_dev(mmc), ...);
> +
> + return ret;
> +}
> +
> +static int sdhci_msm_set_vqmmc(struct sdhci_msm_host *msm_host,
> + struct mmc_host *mmc, int level)
vqmmc_enabled is a bool and "level" sounds like an int with several
possible values. So please make level bool here as well, to make it
easer to read..
> +{
> + int load, ret;
> + struct mmc_ios ios;
> +
> + if (IS_ERR(mmc->supply.vqmmc) ||
> + (mmc->ios.power_mode == MMC_POWER_UNDEFINED) ||
> + (msm_host->vqmmc_enabled == level))
> + return 0;
> +
> + if (msm_host->vqmmc_load) {
> + load = level ? msm_host->vqmmc_load : 0;
> + ret = regulator_set_load(mmc->supply.vqmmc, load);
Since v5.0 the "load" of a regulator consumer is only taken into
consideration if the consumer is enabled. So given that you're toggling
the regulator below there's no need to change this here.
Just specify the "active load" at probe time.
> + if (ret)
> + goto out;
> + }
> +
> + /*
> + * The IO voltage regulator may not always support a voltage close to
> + * vdd. Set IO voltage based on capability of the regulator.
> + */
Is this comment related to the if/else-if inside the conditional? If so
please move it one line down.
> + if (level) {
> + if (msm_host->caps_0 & CORE_3_0V_SUPPORT)
> + ios.signal_voltage = MMC_SIGNAL_VOLTAGE_330;
> + else if (msm_host->caps_0 & CORE_1_8V_SUPPORT)
> + ios.signal_voltage = MMC_SIGNAL_VOLTAGE_180;
Please add a space here, to indicate that the if statement on the next
line is unrelated to the if/elseif above.
> + if (msm_host->caps_0 & CORE_VOLT_SUPPORT) {
> + pr_debug("%s: %s: setting signal voltage: %d\n",
> + mmc_hostname(mmc), __func__,
> + ios.signal_voltage);
I strongly believe you should replace these debug prints with
tracepoints, throughout the driver.
> + ret = mmc_regulator_set_vqmmc(mmc, &ios);
> + if (ret < 0)
> + goto out;
> + }
> + ret = regulator_enable(mmc->supply.vqmmc);
> + } else {
> + ret = regulator_disable(mmc->supply.vqmmc);
> + }
Given that you don't need to regulator_set_load() this function is now
just one large if/else condition on a constant passed as an argument.
Please split it into sdhci_msm_enable_vqmmc() and
sdhci_msm_disable_vqmmc().
> +out:
> + if (ret)
> + pr_err("%s: vqmmc failed: %d\n", mmc_hostname(mmc), ret);
I think it would be useful to know if this error came from
mmc_regulator_set_vqmmc() or regulator_enable() - or
regulator_disable().
So please move this up and add some context in the error message, and
please use dev_err().
> + else
> + msm_host->vqmmc_enabled = level;
> +
> + return ret;
> +}
> +
> static inline void sdhci_msm_init_pwr_irq_wait(struct sdhci_msm_host *msm_host)
> {
> init_waitqueue_head(&msm_host->pwr_irq_wait);
> @@ -1401,8 +1478,9 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> + struct mmc_host *mmc = host->mmc;
> u32 irq_status, irq_ack = 0;
> - int retry = 10;
> + int retry = 10, ret = 0;
There's no need to initialize ret, in all occasions it's assigned before
being read.
> u32 pwr_state = 0, io_level = 0;
> u32 config;
> const struct sdhci_msm_offset *msm_offset = msm_host->offset;
> @@ -1438,14 +1516,35 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
>
> /* Handle BUS ON/OFF*/
> if (irq_status & CORE_PWRCTL_BUS_ON) {
> - pwr_state = REQ_BUS_ON;
> - io_level = REQ_IO_HIGH;
> - irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
> + ret = sdhci_msm_set_vmmc(msm_host, mmc, 1);
> + if (!ret)
> + ret = sdhci_msm_set_vqmmc(msm_host, mmc, 1);
I find this quite complex to follow. Wouldn't it be cleaner to retain
the 4 checks on irq_status as they are and then before the writel of
irq_ack check pwr_state and io_level and call sdhci_msm_set_{vmmc,vqmmc}
accordingly?
> +
> + if (!ret) {
> + pwr_state = REQ_BUS_ON;
> + io_level = REQ_IO_HIGH;
> + irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
> + } else {
> + pr_err("%s: BUS_ON req failed(%d). irq_status: 0x%08x\n",
> + mmc_hostname(mmc), ret, irq_status);
You already printed that this failed in sdhci_msm_set_{vmmc,vqmmc}, no
need to print again.
> + irq_ack |= CORE_PWRCTL_BUS_FAIL;
> + sdhci_msm_set_vmmc(msm_host, mmc, 0);
> + }
> }
> if (irq_status & CORE_PWRCTL_BUS_OFF) {
> - pwr_state = REQ_BUS_OFF;
> - io_level = REQ_IO_LOW;
> - irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
> + ret = sdhci_msm_set_vmmc(msm_host, mmc, 0);
> + if (!ret)
> + ret = sdhci_msm_set_vqmmc(msm_host, mmc, 0);
> +
> + if (!ret) {
> + pwr_state = REQ_BUS_OFF;
> + io_level = REQ_IO_LOW;
> + irq_ack |= CORE_PWRCTL_BUS_SUCCESS;
> + } else {
> + pr_err("%s: BUS_ON req failed(%d). irq_status: 0x%08x\n",
> + mmc_hostname(mmc), ret, irq_status);
> + irq_ack |= CORE_PWRCTL_BUS_FAIL;
> + }
> }
> /* Handle IO LOW/HIGH */
> if (irq_status & CORE_PWRCTL_IO_LOW) {
> @@ -1457,6 +1556,15 @@ static void sdhci_msm_handle_pwr_irq(struct sdhci_host *host, int irq)
> irq_ack |= CORE_PWRCTL_IO_SUCCESS;
> }
>
> + if (io_level && !IS_ERR(mmc->supply.vqmmc) && !pwr_state) {
> + ret = mmc_regulator_set_vqmmc(mmc, &mmc->ios);
Didn't you already call this through sdhci_msm_set_vqmmc()?
> + if (ret < 0)
> + pr_err("%s: IO_level setting failed(%d). signal_voltage: %d, vdd: %d irq_status: 0x%08x\n",
> + mmc_hostname(mmc), ret,
> + mmc->ios.signal_voltage, mmc->ios.vdd,
> + irq_status);
> + }
> +
> /*
> * The driver has to acknowledge the interrupt, switch voltages and
> * report back if it succeded or not to this register. The voltage
> @@ -1833,6 +1941,91 @@ static void sdhci_msm_reset(struct sdhci_host *host, u8 mask)
> sdhci_reset(host, mask);
> }
>
> +static int sdhci_msm_register_vreg(struct sdhci_msm_host *msm_host)
> +{
> + int ret = 0;
No need to initialize ret, first use is an assignment.
> + struct mmc_host *mmc = msm_host->mmc;
> +
> + ret = mmc_regulator_get_supply(msm_host->mmc);
> + if (ret)
> + return ret;
> + device_property_read_u32(&msm_host->pdev->dev,
> + "vmmc-max-load-microamp",
> + &msm_host->vmmc_load);
> + device_property_read_u32(&msm_host->pdev->dev,
> + "vqmmc-max-load-microamp",
> + &msm_host->vqmmc_load);
These properties are not documented. Do they vary enough to mandate
being read from DT or could they simply be approximated by a define
instead?
> +
> + sdhci_msm_set_regulator_caps(msm_host);
> + mmc->ios.power_mode = MMC_POWER_UNDEFINED;
> +
> + return 0;
> +
> +}
> +
> +static int sdhci_msm_start_signal_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + u16 ctrl;
> +
> + /*
> + * Signal Voltage Switching is only applicable for Host Controllers
> + * v3.00 and above.
> + */
> + if (host->version < SDHCI_SPEC_300)
> + return 0;
> +
> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +
> + switch (ios->signal_voltage) {
> + case MMC_SIGNAL_VOLTAGE_330:
> + if (!(host->flags & SDHCI_SIGNALING_330))
> + return -EINVAL;
> + /* Set 1.8V Signal Enable in the Host Control2 register to 0 */
> + ctrl &= ~SDHCI_CTRL_VDD_180;
> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> +
> + /* 3.3V regulator output should be stable within 5 ms */
What mechanism ensures that the readw won't return withing 5ms from the
writew above?
> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + if (!(ctrl & SDHCI_CTRL_VDD_180))
> + return 0;
> +
> + pr_warn("%s: 3.3V regulator output did not became stable\n",
> + mmc_hostname(mmc));
> +
> + return -EAGAIN;
The body of the 330 and 180 cases are quite similar, can you perhaps
deal with those after the switch, once?
> + case MMC_SIGNAL_VOLTAGE_180:
> + if (!(host->flags & SDHCI_SIGNALING_180))
> + return -EINVAL;
> +
> + /*
> + * Enable 1.8V Signal Enable in the Host Control2
> + * register
> + */
> + ctrl |= SDHCI_CTRL_VDD_180;
> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> +
> + /* 1.8V regulator output should be stable within 5 ms */
> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + if (ctrl & SDHCI_CTRL_VDD_180)
> + return 0;
> +
> + pr_warn("%s: 1.8V regulator output did not became stable\n",
> + mmc_hostname(mmc));
> +
> + return -EAGAIN;
> + case MMC_SIGNAL_VOLTAGE_120:
> + if (!(host->flags & SDHCI_SIGNALING_120))
> + return -EINVAL;
> + return 0;
> + default:
> + /* No signal voltage switch required */
> + return 0;
> + }
> +
Empty line.
Regards,
Bjorn
> +}
> +
> static const struct sdhci_msm_variant_ops mci_var_ops = {
> .msm_readl_relaxed = sdhci_msm_mci_variant_readl_relaxed,
> .msm_writel_relaxed = sdhci_msm_mci_variant_writel_relaxed,
> @@ -1880,6 +2073,7 @@ static void sdhci_msm_reset(struct sdhci_host *host, u8 mask)
> .write_w = sdhci_msm_writew,
> .write_b = sdhci_msm_writeb,
> .irq = sdhci_msm_cqe_irq,
> + .set_power = sdhci_set_power_noreg,
> };
>
> static const struct sdhci_pltfm_data sdhci_msm_pdata = {
> @@ -2072,6 +2266,10 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> if (core_major == 1 && core_minor >= 0x49)
> msm_host->updated_ddr_cfg = true;
>
> + ret = sdhci_msm_register_vreg(msm_host);
> + if (ret)
> + goto clk_disable;
> +
> /*
> * Power on reset state may trigger power irq if previous status of
> * PWRCTL was either BUS_ON or IO_HIGH_V. So before enabling pwr irq
> @@ -2116,6 +2314,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> MSM_MMC_AUTOSUSPEND_DELAY_MS);
> pm_runtime_use_autosuspend(&pdev->dev);
>
> + host->mmc_host_ops.start_signal_voltage_switch =
> + sdhci_msm_start_signal_voltage_switch;
> host->mmc_host_ops.execute_tuning = sdhci_msm_execute_tuning;
> if (of_property_read_bool(node, "supports-cqe"))
> ret = sdhci_msm_cqe_add_host(host, pdev);
> @@ -2123,7 +2323,6 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> ret = sdhci_add_host(host);
> if (ret)
> goto pm_runtime_disable;
> - sdhci_msm_set_regulator_caps(msm_host);
>
> pm_runtime_mark_last_busy(&pdev->dev);
> pm_runtime_put_autosuspend(&pdev->dev);
> --
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>
^ permalink raw reply
* Re: [PATCH 2/3] pinctrl: mchp-sgpio: Add pinctrl driver for Microsemi Serial GPIO
From: Lars Povlsen @ 2020-05-18 19:44 UTC (permalink / raw)
To: Randy Dunlap
Cc: Lars Povlsen, SoC Team, Linus Walleij,
Microchip Linux Driver Support, devicetree, linux-gpio,
linux-arm-kernel, linux-kernel, Alexandre Belloni
In-Reply-To: <fb104436-69c1-3f5a-85d9-af0aee35547f@infradead.org>
Randy Dunlap writes:
> On 5/13/20 7:11 AM, Lars Povlsen wrote:
>> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
>> index 834c59950d1cf..2b0e9021fd7e0 100644
>> --- a/drivers/pinctrl/Kconfig
>> +++ b/drivers/pinctrl/Kconfig
>> @@ -396,6 +396,23 @@ config PINCTRL_OCELOT
>> select OF_GPIO
>> select REGMAP_MMIO
>>
>> +config PINCTRL_MSCC_SGPIO
>> + bool "Pinctrl driver for Microsemi Serial GPIO"
>> + depends on OF
>> + depends on HAS_IOMEM
>> + select GPIOLIB
>> + select GENERIC_PINCONF
>> + select GENERIC_PINCTRL_GROUPS
>> + select GENERIC_PINMUX_FUNCTIONS
>> + select OF_GPIO
>> + help
>> + Support for the VCoreIII SoC serial GPIO device. By using a
>
> Line above should be indented with one tab + 2 spaces...
> like the lines below.
>
Well spotted...
>> + serial interface, the SIO controller significantly extends
>> + the number of available GPIOs with a minimum number of
>> + additional pins on the device. The primary purpose of the
>> + SIO controller is to connect control signals from SFP
>> + modules and to act as an LED controller.
>> +
>
> thanks.
Thank you for your comments.
--
Lars Povlsen,
Microchip
^ permalink raw reply
* Re: [PATCH] dt-bindings: clock: renesas: mstp: Convert to json-schema
From: Geert Uytterhoeven @ 2020-05-18 19:36 UTC (permalink / raw)
To: Rob Herring
Cc: Michael Turquette, Stephen Boyd,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-clk, Linux-Renesas
In-Reply-To: <20200518161801.GA9983@bogus>
Hi Rob,
On Mon, May 18, 2020 at 6:18 PM Rob Herring <robh@kernel.org> wrote:
> On Fri, May 08, 2020 at 12:03:21PM +0200, Geert Uytterhoeven wrote:
> > Convert the Renesas Clock Pulse Generator (CPG) Module Stop (MSTP)
> > Clocks Device Tree binding documentation to json-schema.
> >
> > Drop R-Car Gen2 compatible values, which were obsoleted by the unified
> > "Renesas Clock Pulse Generator / Module Standby and Software Reset" DT
> > bindings.
> > Replace the obsolete example for R-Car H2 by an example that is still
> > valid.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.yaml
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/r8a73a4-clock.h>
> > + mstp2_clks: mstp2_clks@e6150138 {
> > + compatible = "renesas,r8a73a4-mstp-clocks",
> > + "renesas,cpg-mstp-clocks";
> > + reg = <0xe6150138 0 4>, <0xe6150040 0 4>;
>
> Size is 2 cells?
Oops, forgot to drop the zeroes. Fixed.
> With that fixed,
>
> Reviewed-by: Rob Herring <robh@kernel.org>
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
* Re: [PATCH v2 2/6] dt-bindings: dma: dw: Add max burst transaction length property
From: Serge Semin @ 2020-05-18 19:30 UTC (permalink / raw)
To: Rob Herring
Cc: Serge Semin, Vinod Koul, Andy Shevchenko, Viresh Kumar,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Dan Williams, linux-mips, dmaengine, devicetree,
linux-kernel
In-Reply-To: <20200518173003.GA13764@bogus>
On Mon, May 18, 2020 at 11:30:03AM -0600, Rob Herring wrote:
> On Sun, May 17, 2020 at 08:47:39PM +0300, Serge Semin wrote:
> > On Fri, May 15, 2020 at 02:11:13PM +0300, Serge Semin wrote:
> > > On Fri, May 15, 2020 at 04:26:58PM +0530, Vinod Koul wrote:
> > > > On 15-05-20, 13:51, Andy Shevchenko wrote:
> > > > > On Fri, May 15, 2020 at 11:39:11AM +0530, Vinod Koul wrote:
> > > > > > On 12-05-20, 15:38, Andy Shevchenko wrote:
> > > > > > > On Tue, May 12, 2020 at 02:49:46PM +0300, Serge Semin wrote:
> > > > > > > > On Tue, May 12, 2020 at 12:08:04PM +0300, Andy Shevchenko wrote:
> > > > > > > > > On Tue, May 12, 2020 at 12:35:31AM +0300, Serge Semin wrote:
> > > > > > > > > > On Tue, May 12, 2020 at 12:01:38AM +0300, Andy Shevchenko wrote:
> > > > > > > > > > > On Mon, May 11, 2020 at 11:05:28PM +0300, Serge Semin wrote:
> > > > > > > > > > > > On Fri, May 08, 2020 at 02:12:42PM +0300, Andy Shevchenko wrote:
> > > > > > > > > > > > > On Fri, May 08, 2020 at 01:53:00PM +0300, Serge Semin wrote:
> > > > >
> > > > > ...
> > > > >
> > > > > > > I leave it to Rob and Vinod.
> > > > > > > It won't break our case, so, feel free with your approach.
> > > > > >
> > > > > > I agree the DT is about describing the hardware and looks like value of
> > > > > > 1 is not allowed. If allowed it should be added..
> > > > >
> > > > > It's allowed at *run time*, it's illegal in *pre-silicon stage* when
> > > > > synthesizing the IP.
> > > >
> > > > Then it should be added ..
> > >
> > > Vinod, max-burst-len is "MAXimum" burst length not "run-time or current or any
> > > other" burst length. It's a constant defined at the IP-core synthesis stage and
> > > according to the Data Book, MAX burst length can't be 1. The allowed values are
> > > exactly as I described in the binding [4, 8, 16, 32, ...]. MAX burst length
> > > defines the upper limit of the run-time burst length. So setting it to 1 isn't
> > > about describing a hardware, but using DT for the software convenience.
> > >
> > > -Sergey
> >
> > Vinod, to make this completely clear. According to the DW DMAC data book:
> > - In general, run-time parameter of the DMA transaction burst length (set in
> > the SRC_MSIZE/DST_MSIZE fields of the channel control register) may belong
> > to the set [1, 4, 8, 16, 32, 64, 128, 256].
> > - Actual upper limit of the burst length run-time parameter is limited by a
> > constant defined at the IP-synthesize stage (it's called DMAH_CHx_MAX_MULT_SIZE)
> > and this constant belongs to the set [4, 8, 16, 32, 64, 128, 256]. (See, no 1
> > in this set).
> >
> > So the run-time burst length in a case of particular DW DMA controller belongs
> > to the range:
> > 1 <= SRC_MSIZE <= DMAH_CHx_MAX_MULT_SIZE
> > and
> > 1 <= DST_MSIZE <= DMAH_CHx_MAX_MULT_SIZE
> >
> > See. No mater which DW DMA controller we get each of them will at least support
> > the burst length of 1 and 4 transfer words. This is determined by design of the
> > DW DMA controller IP since DMAH_CHx_MAX_MULT_SIZE constant set starts with 4.
> >
> > In this patch I suggest to add the max-burst-len property, which specifies
> > the upper limit for the run-time burst length. Since the maximum burst length
> > capable to be set to the SRC_MSIZE/DST_MSIZE fields of the DMA channel control
> > register is determined by the DMAH_CHx_MAX_MULT_SIZE constant (which can't be 1
> > by the DW DMA IP design), max-burst-len property as being also responsible for
> > the maximum burst length setting should be associated with DMAH_CHx_MAX_MULT_SIZE
> > thus should belong to the same set [4, 8, 16, 32, 64, 128, 256].
> >
> > So 1 shouldn't be in the enum of the max-burst-len property constraint, because
> > hardware doesn't support such limitation by design, while setting 1 as
> > max-burst-len would mean incorrect description of the DMA controller.
> >
> > Vinod, could you take a look at the info I provided above and say your final word
> > whether 1 should be really allowed to be in the max-burst-len enum constraints?
> > I'll do as you say in the next version of the patchset.
>
> I generally think the synthesis time IP configuration should be implied
> by the compatible string which is why we have SoC specific compatible
> strings (Of course I dream for IP vendors to make all that discoverable
> which is only occasionally the case). There are exceptions to this. If
> one SoC has the same IP configured in different ways, then we'd probably
> have properties for the differences.
Hm, AFAIU from what you said the IP configuration specific to a particular
SoC must be determined by the compatible string and that configuration parameters
should be hidden somewhere in the driver internals for instance in the platform
data structure. In case if there are several versions of the same IP are embedded
into the SoC, then the differences can be described by the DT properties. Right?
If I am right, then that's weird. A lot of the currently available platforms (and
drivers) don't follow that rule and just specify the generic IP compatible string
and describe their IP synthesis parameters by the DT properties.
>
> As to whether h/w configuration is okay in DT, the answer is yes. The
> question is whether it is determined by SoC, board, OS and also who
> would set it and how often. Something tuned per board and independent of
> the OS/user is the ideal example.
So does this mean that I have to allow the max-burst-len property to be 1 even
though in accordance with the DW DMA Data Book the upper limit of the
burst-length will never be 1, but will always start with 4? By allowing the
upper limit to be 1 we wouldn't provide the h/w configuration (hardware has
already been configured with maximum burst length parameter DMAH_CHx_MAX_MULT_SIZE
on the IP synthesis stage), but would setup an artificial constraints on the
maximum allowed burst length. Are you ok with this and 1 should be permitted
anyway?
-Sergey
>
> Rob
^ permalink raw reply
* [PATCH v8 2/2] Device tree bindings for TI HDC20x0 humidity and temperature sensors
From: Eugene Zalkonnikov @ 2020-05-18 19:22 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Hartmut Knaack, Lars-Peter Clausen, development@norphonic.com,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
Mark Rutland
In-Reply-To: <5489E47F-47D4-421A-A7EA-E11B0205A842@norphonic.com>
Device tree bindings for HDC2010/HDC2080 driver.
Signed-off-by: Eugene Zaikonnikov <eugene.zaikonnikov@norphonic.com>
diff -uprN linux-5.3.8/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml linux-5.3.8_docs/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml
--- linux-5.3.8/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml 1970-01-01 01:00:00.000000000 +0100
+++ linux-5.3.8_docs/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml 2020-04-24 17:50:58.213007228 +0200
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/humidity/ti,hdc2010.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HDC2010/HDC2080 humidity and temperature iio sensors
+
+maintainers:
+ - Eugene Zaikonnikov <eugene.zaikonnikov@norophonic.com>
+
+description: |
+ Relative humidity and tempereature sensors on I2C bus
+
+ Datasheets are available at:
+ http://www.ti.com/product/HDC2010/datasheet
+ http://www.ti.com/product/HDC2080/datasheet
+
+properties:
+ compatible:
+ enum:
+ - ti,hdc2010
+ - ti,hdc2080
+
+ vdd-supply:
+ maxItems: 1
+
+required:
+ - compatible
+
+examples:
+ - |
+ i2c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ humidity@40 {
+ compatible = "ti,hdc2010";
+ reg = <0x40>;
+ };
+ };
^ permalink raw reply
* Re: [PATCH 1/2] arm64: dts: qcom: sc7180: Support ETMv4 power management
From: Sai Prakash Ranjan @ 2020-05-18 19:08 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Mathieu Poirier, Suzuki K Poulose, Mike Leach, devicetree,
Rob Herring, Andy Gross, Stephen Boyd, linux-arm-kernel,
linux-kernel, linux-arm-msm, coresight
In-Reply-To: <20200518185124.GG2165@builder.lan>
Hi Bjorn,
On 2020-05-19 00:21, Bjorn Andersson wrote:
> On Fri 15 May 03:55 PDT 2020, Sai Prakash Ranjan wrote:
[...]
>>
>>
>> The previous version of this patch in QCOM tree seems to have added
>> the
>> property to replicator node instead of etm7 node, can you please drop
>> that from the tree and apply this one?
>>
>
> I'm not able to replace the old commit without rewriting the history of
> the branch. So I've applied a patch ontop of the branch to fix this up
> instead.
>
> Please review the branch and let me know if there's any issues.
>
Thanks for the patch, I checked the branch and its good.
Sorry, I should have sent a patch on top of the old one
instead of repost.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH 1/2] arm64: dts: qcom: sc7180: Support ETMv4 power management
From: Bjorn Andersson @ 2020-05-18 18:51 UTC (permalink / raw)
To: Sai Prakash Ranjan
Cc: Mathieu Poirier, Suzuki K Poulose, Mike Leach, devicetree,
Rob Herring, Andy Gross, Stephen Boyd, linux-arm-kernel,
linux-kernel, linux-arm-msm, coresight
In-Reply-To: <56a5563205da61c47eb4f8bbf6120e28@codeaurora.org>
On Fri 15 May 03:55 PDT 2020, Sai Prakash Ranjan wrote:
> Hi Bjorn,
>
Hi Sai,
> On 2020-05-15 16:21, Sai Prakash Ranjan wrote:
> > Now that deep idle states are properly supported on SC7180,
> > we need to add "coresight-loses-context-with-cpu" property
> > to avoid failure of trace session because of losing context
> > on entering deep idle states.
> >
> > Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> > ---
> >
> > Resending this because the last patch sent here -
> > https://lore.kernel.org/patchwork/patch/1230367/
> > seems to have added "coresight-loses-context-with-cpu" to
> > replicator node instead of etm7 node.
> >
> > ---
> > arch/arm64/boot/dts/qcom/sc7180.dtsi | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > index 4069bb1c93af..8b3707347547 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> > @@ -1656,6 +1656,7 @@
> >
> > clocks = <&aoss_qmp>;
> > clock-names = "apb_pclk";
> > + arm,coresight-loses-context-with-cpu;
> >
> > out-ports {
> > port {
> > @@ -1674,6 +1675,7 @@
> >
> > clocks = <&aoss_qmp>;
> > clock-names = "apb_pclk";
> > + arm,coresight-loses-context-with-cpu;
> >
> > out-ports {
> > port {
> > @@ -1692,6 +1694,7 @@
> >
> > clocks = <&aoss_qmp>;
> > clock-names = "apb_pclk";
> > + arm,coresight-loses-context-with-cpu;
> >
> > out-ports {
> > port {
> > @@ -1710,6 +1713,7 @@
> >
> > clocks = <&aoss_qmp>;
> > clock-names = "apb_pclk";
> > + arm,coresight-loses-context-with-cpu;
> >
> > out-ports {
> > port {
> > @@ -1728,6 +1732,7 @@
> >
> > clocks = <&aoss_qmp>;
> > clock-names = "apb_pclk";
> > + arm,coresight-loses-context-with-cpu;
> >
> > out-ports {
> > port {
> > @@ -1746,6 +1751,7 @@
> >
> > clocks = <&aoss_qmp>;
> > clock-names = "apb_pclk";
> > + arm,coresight-loses-context-with-cpu;
> >
> > out-ports {
> > port {
> > @@ -1764,6 +1770,7 @@
> >
> > clocks = <&aoss_qmp>;
> > clock-names = "apb_pclk";
> > + arm,coresight-loses-context-with-cpu;
> >
> > out-ports {
> > port {
> > @@ -1782,6 +1789,7 @@
> >
> > clocks = <&aoss_qmp>;
> > clock-names = "apb_pclk";
> > + arm,coresight-loses-context-with-cpu;
> >
> > out-ports {
> > port {
>
>
> The previous version of this patch in QCOM tree seems to have added the
> property to replicator node instead of etm7 node, can you please drop
> that from the tree and apply this one?
>
I'm not able to replace the old commit without rewriting the history of
the branch. So I've applied a patch ontop of the branch to fix this up
instead.
Please review the branch and let me know if there's any issues.
Regards,
Bjorn
^ permalink raw reply
* Re: [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support
From: Jonathan Albrieux @ 2020-05-18 18:46 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linux Kernel Mailing List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Greg Kroah-Hartman, Hartmut Knaack, Jonathan Cameron,
Lars-Peter Clausen, Linus Walleij,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Steve Winslow, Thomas Gleixner, Jonathan Cameron
In-Reply-To: <CAHp75VcJ+LU_T4jRVNDBed50xaALDLNC_8brxszv5FpUGHF7+A@mail.gmail.com>
On Mon, May 18, 2020 at 08:51:29PM +0300, Andy Shevchenko wrote:
> On Mon, May 18, 2020 at 8:43 PM Jonathan Albrieux
> <jonathan.albrieux@gmail.com> wrote:
> >
> > On Mon, May 18, 2020 at 07:43:17PM +0300, Andy Shevchenko wrote:
> > > On Mon, May 18, 2020 at 06:01:20PM +0200, Jonathan Albrieux wrote:
> > > > On Mon, May 18, 2020 at 05:55:51PM +0300, Andy Shevchenko wrote:
> > > > > On Mon, May 18, 2020 at 4:38 PM Jonathan Albrieux
> > > > > <jonathan.albrieux@gmail.com> wrote:
> > > > >
> > > > > > + gpiod_set_value_cansleep(data->reset_gpiod, 1);
> > > > >
> > > > > (1)
> > > > >
> > > > > ...
> > > > >
> > > > > > + /*
> > > > > > + * If reset pin is provided then will be set to high on power on
> > > > > > + * and to low on power off according to AK09911 datasheet
> > > > > > + */
> > > > >
> > > > > Wording is confusing, perhaps you have to use 'asserted / deasserted'.
> > > >
> > > > Thank you for the suggestion, I'll be working on rewording as soon as
> > > > possible.
> > > >
> > > > > Btw, in (1) it's also "high" (asserted). I barely understand how it's
> > > > > supposed to work in all cases?
> > > > >
> > > > > > + reset_gpiod = devm_gpiod_get_optional(&client->dev,
> > > > > > + "reset", GPIOD_OUT_HIGH);
> > > > > > + if (IS_ERR(reset_gpiod))
> > > > > > + return PTR_ERR(reset_gpiod);
> > > > >
> > > >
> > > > I'm sorry but I'm not sure about what you mean by saying all cases.
> > > > Currently I'm testing this driver on a msm8916 device having AK09911
> > > > magnetometer. At the current stage the driver is failing on probe
> > > > because reset pin is not connected to VID (as datasheet requires in case
> > > > of pin not being used). In case of reset pin not asserted, register's
> > > > reset is triggered resulting in empty registers, leading to probe fail.
> > > > For this reason pin is asserted during power on in order to have
> > > > informations in registers and deasserted before power off triggering
> > > > a reset.
> > > >
> > > > A workaround that gets AK09911 working on device is by setting the
> > > > reset pin always high on device tree. This way registers gets reset by
> > > > a Power On Reset circuit autonomously and reset pin never triggers the
> > > > reset.
> > >
> > > You need to distinguish electrical level from logical (GPIO flag defines
> > > logical). So, I'm talking about active-high vs. active-low case.
> > >
> > > Now I re-read above, and see that here you assert the reset signal. But where
> > > is desertion?
> >
> > Oh I see, I'll try explaining by points the proposed approach:
> > - reset pin is active low
> > - during power on gpio is set to 0 so the reset pin is high, thus no reset
>
> deasserted
>
> > - during power off gpio is set to 1 so the reset pin becomes low, thus resetting
>
> asserted
>
> > this is a possible solution but maybe there are other ways to achieve that,
> > do you have suggestions on how to get a better approach for solving this issue?
>
> I see now, that at requesting reset you wanted to chip be in reset
> state (asserted) till driver calls power_on().
>
> Seems everything you done is correct. Just correct terminology, please.
Will surely do, thank you!
> --
> With Best Regards,
> Andy Shevchenko
Best regards,
Jonathan Albrieux
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: sc16is7xx: Add flag to activate IrDA mode
From: Daniel Mack @ 2020-05-18 18:41 UTC (permalink / raw)
To: Rob Herring; +Cc: devicetree, linux-serial, gregkh, jslaby, pascal.huerst
In-Reply-To: <20200518180853.GA18566@bogus>
On 5/18/20 8:08 PM, Rob Herring wrote:
> On Fri, May 08, 2020 at 04:37:54PM +0200, Daniel Mack wrote:
>> From: Pascal Huerst <pascal.huerst@gmail.com>
>>
>> This series of uart controllers is able to work in IrDA mode.
>> This adds a flag to the device tree to enable that feature if needed.
>>
>> Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com>
>> ---
>> Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
>> index c1091a923a89..9317c320c82c 100644
>> --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
>> +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
>> @@ -21,6 +21,8 @@ Optional properties:
>> the second cell is used to specify the GPIO polarity:
>> 0 = active high,
>> 1 = active low.
>> +- linux,irda-mode-port-0: Use IrDA mode on port #0
>> +- linux,irda-mode-port-1: Use IrDA mode on port #1 (for dual-port devices only)
>>
>> Example:
>> sc16is750: sc16is750@51 {
>> @@ -55,6 +57,8 @@ Optional properties:
>> the second cell is used to specify the GPIO polarity:
>> 0 = active high,
>> 1 = active low.
>> +- linux,irda-mode-port-0: Use IrDA mode on port #0
>> +- linux,irda-mode-port-1: Use IrDA mode on port #1 (for dual-port devices only)
>
> How about an array of 1 or 2 entries instead?
Yes, that would be an alternative. We just weren't sure what type the
elements of the array would have. Could you point to an existing example?
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH v7 2/4] usb: dwc3: qcom: Add interconnect support in dwc3 driver
From: Bjorn Andersson @ 2020-05-18 18:35 UTC (permalink / raw)
To: Felipe Balbi
Cc: Georgi Djakov, Matthias Kaehlcke, Sandeep Maheswaram, Andy Gross,
Greg Kroah-Hartman, Rob Herring, Mark Rutland, Stephen Boyd,
Doug Anderson, linux-arm-msm, linux-usb, devicetree, linux-kernel,
Manu Gautam, Chandana Kishori Chiluveru, Viresh Kumar
In-Reply-To: <87r1vl3e42.fsf@kernel.org>
On Thu 14 May 23:29 PDT 2020, Felipe Balbi wrote:
>
> Hi,
>
> Georgi Djakov <georgi.djakov@linaro.org> writes:
> >>>>>> Sandeep Maheswaram <sanm@codeaurora.org> writes:
> >>>>>>> +static int dwc3_qcom_interconnect_init(struct dwc3_qcom *qcom)
> >>>>>>> +{
> >>>>>>> + struct device *dev = qcom->dev;
> >>>>>>> + int ret;
> >>>>>>> +
> >>>>>>> + if (!device_is_bound(&qcom->dwc3->dev))
> >>>>>>> + return -EPROBE_DEFER;
> >>>>>>
> >>>>>> this breaks allmodconfig. I'm dropping this series from my queue for
> >>>>>> this merge window.
> >>>>>
> >>>>> Sorry, I meant this patch ;-)
> >>>>
> >>>> I guess that's due to INTERCONNECT being a module. There is currently a
> >>>
> >>> I believe it's because of this:
> >>> ERROR: modpost: "device_is_bound" [drivers/usb/dwc3/dwc3-qcom.ko] undefined!
> >>>
> >>>> discussion about this with Viresh and Georgi in response to another
> >>>> automated build failure. Viresh suggests changing CONFIG_INTERCONNECT
> >>>> from tristate to bool, which seems sensible to me given that interconnect
> >>>> is a core subsystem.
> >>>
> >>> The problem you are talking about would arise when INTERCONNECT=m and
> >>> USB_DWC3_QCOM=y and it definitely exists here and could be triggered with
> >>> randconfig build. So i suggest to squash also the diff below.
> >>>
> >>> Thanks,
> >>> Georgi
> >>>
> >>> ---8<---
> >>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> >>> index 206caa0ea1c6..6661788b1a76 100644
> >>> --- a/drivers/usb/dwc3/Kconfig
> >>> +++ b/drivers/usb/dwc3/Kconfig
> >>> @@ -129,6 +129,7 @@ config USB_DWC3_QCOM
> >>> tristate "Qualcomm Platform"
> >>> depends on ARCH_QCOM || COMPILE_TEST
> >>> depends on EXTCON || !EXTCON
> >>> + depends on INTERCONNECT || !INTERCONNECT
> >>
> >> I would prefer to see a patch adding EXPORT_SYMBOL_GPL() to device_is_bound()
> >
> > Agree, but just to clarify, that these are two separate issues that need to
> > be fixed. The device_is_bound() is the first one and USB_DWC3_QCOM=y combined
> > with INTERCONNECT=m is the second one.
>
> If INTERCONNECT=m, QCOM3 shouldn't be y. I think the following is
> enough:
>
> depends on INTERCONNECT=y || INTERCONNECT=USB_DWC3_QCOM
>
This misses the case where INTERCONNECT=n and USB_DWC3_QCOM=[ym] which
I don't see a reason for breaking.
But if only INTERCONNECT where a bool, then we don't need to specify a
depends on, because it will either be there, or the stubs will.
We've come to this conclusion in a lot of different frameworks and I
don't see why we should do this differently with INTERCONNECT.
Regards,
Bjorn
^ permalink raw reply
* Re: [RFC v1 2/3] drivers: nvmem: Add driver for QTI qfprom-efuse support
From: Doug Anderson @ 2020-05-18 18:31 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Ravi Kumar Bokka (Temp), Rob Herring, LKML,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rajendra Nayak, Sai Prakash Ranjan, dhavalp, mturney, sparate,
c_rbokka, mkurumel
In-Reply-To: <d76e4eb2-fa6a-0b76-3912-83bce678bc96@linaro.org>
Hi,
On Mon, May 18, 2020 at 3:45 AM Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
> On 18/05/2020 11:39, Ravi Kumar Bokka (Temp) wrote:
> >
> > Based on the compatible, do i need to separate probe function for
> > qfprom-efuse and maintain separate nvmem object to register nvmem
> > framework. Is this what you are suggesting to implementing this in to
> > one existing driver?
>
> Yes for same driver we should add new compatible string and add support
> to this in existing qfprom driver.
> Ideally we should allocate nvmem_config object at probe with different
> parameters based on compatible string.
I wish I had better documentation for exactly what was in the SoC
instead of the heavily redacted stuff Qualcomm provides. Really the
answer here is: how do you best describe the hardware? OK, so I just
spent the past hour or so trying to patch together all the bits and
fragments that Qualcomm provided me. Just like a scavenger hunt!
Fun! The best I can patch together is that there is a single QFPROM
with these ranges:
0x00780000 - 0x007800FF
QFPROM HW registers, range 1/2
0x00780120 - 0x007808FF
QFPROM "raw" space
0x00782000 - 0x007820FF
QFPROM HW registers, range 2/2
0x00784120 - 0x007848FF
QFPROM "corrected" space
0x00786000 - 0x00786FFF
QFPROM memory range that I don't really understand and maybe we don't
worry about right now?
Did I get that right? If so, is there a prize for winning the scavenger hunt?
---
If so then, IMO, it wouldn't be insane to actually keep it as two
drivers and two device tree nodes, as you've done. I'd defer to
Srinivas and Rob Herring, though. The existing driver would be a
read-only driver and provide access to the "corrected" versions of all
the registers. Its node would have "#address-cells = <1>" and
"#size-cells = <1>" because it's expected that other drivers might
need to refer to data stored here.
Your new driver would be read-write and provide access to the "raw"
values. A read from your new driver would not necessarily equal a
read from the old driver if the FEC (forward error correction) kicked
in. Other drivers should never refer to the non-corrected values so
you wouldn't have "#address-cells" and "#size-cells". The only way to
really read or write it would be through sysfs.
It would be super important to document what's happening, of course.
...and ideally name them to make it clearer too.
---
Another alternative (if Srinivas and/or Rob H prefer it) would be to
deprecate the old driver and/or bindings and say that there really
should just be one node and one driver. In that case you'd replace
the old node with:
qfprom@780000 {
compatible = "qcom,sc7180-qfprom-efuse";
reg = <0 0x00780000 0 0x6fff>;
#address-cells = <1>;
#size-cells = <1>;
clocks = <&gcc GCC_SEC_CTRL_CLK_SRC>;
clock-names = "sec";
qusb2p_hstx_trim: hstx-trim-primary@25b {
reg = <0x25b 0x1>;
bits = <1 3>;
};
};
You'd use the of_match_table solution to figure out the relevant
offsets (0x120, 0x2000, 0x4120, 0x6000) for sc7180 and this new driver
would be responsible for being able to read the corrected values and
also for programming. In this case I'm not sure how (assuming it's
valuable) you'd provide read access to the uncorrected data.
-Doug
^ permalink raw reply
* Re: [PATCH v1] dt-bindings: serial: qca,ar9330-uart: Convert to json-schema
From: Rob Herring @ 2020-05-18 18:29 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Pengutronix Kernel Team, Rob Herring, devicetree,
Greg Kroah-Hartman, linux-kernel, linux-serial, linux-mips
In-Reply-To: <20200510051522.6191-1-o.rempel@pengutronix.de>
On Sun, 10 May 2020 07:15:22 +0200, Oleksij Rempel wrote:
> Convert the Qualcomm Atheros AR9330 High-Speed UART
> Device Tree binding documentation to json-schema.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> .../bindings/serial/qca,ar9330-uart.txt | 31 ------------
> .../bindings/serial/qca,ar9330-uart.yaml | 50 +++++++++++++++++++
> 2 files changed, 50 insertions(+), 31 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
> create mode 100644 Documentation/devicetree/bindings/serial/qca,ar9330-uart.yaml
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: n133hse-ea1: Add Innolux N133HSE panel
From: Rob Herring @ 2020-05-18 18:28 UTC (permalink / raw)
To: srk
Cc: Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, David Airlie, Daniel Vetter, Lubomir Rintel,
dri-devel, devicetree, linux-kernel, Sean Cross, Shawn Guo,
Fabio Estevam, Thierry Reding
In-Reply-To: <20200509111834.26335-2-srk@48.io>
On Sat, May 09, 2020 at 01:18:33PM +0200, srk@48.io wrote:
> From: Sean Cross <xobs@kosagi.com>
>
> The Innolux N133HSE panel is a 13.3" 1920x1080 panel that contains an
> integrated backlight, and connects via eDP.
>
> It is used in the Kosagi Novena.
>
> Signed-off-by: Sean Cross <xobs@kosagi.com>
> Signed-off-by: Richard Marko <srk@48.io>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> To: dri-devel@lists.freedesktop.org
> ---
> .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
> index 393ffc6acbba..756bf6a48e8d 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
> @@ -139,6 +139,8 @@ properties:
> - innolux,g121i1-l01
> # Innolux Corporation 12.1" G121X1-L03 XGA (1024x768) TFT LCD panel
> - innolux,g121x1-l03
> + # Innolux Corporation 13.3" FHD (1920x1080) TFT LCD panel
> + - innolux,n133hse-ea1
> # Innolux Corporation 11.6" WXGA (1366x768) TFT LCD panel
> - innolux,n116bge
Alpha-numeric order please.
> # InnoLux 15.6" WXGA TFT LCD panel
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v5 0/6] drm: Prepare to use a GPIO on ti-sn65dsi86 for Hot Plug Detect
From: Sam Ravnborg @ 2020-05-18 18:28 UTC (permalink / raw)
To: Doug Anderson
Cc: LinusW, Stephen Boyd, Laurent Pinchart, Neil Armstrong,
David Airlie, Sandeep Panda, dri-devel, Bjorn Andersson,
Andrzej Hajda, Thierry Reding, Rob Clark, Jeffrey Hugo,
Krzysztof Kozlowski, Bartosz Golaszewski, Andy Gross,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jonas Karlman, linux-arm-msm, open list:GPIO SUBSYSTEM,
Rob Herring, Paul Walmsley, Jernej Skrabec, Stephen Boyd, LKML
In-Reply-To: <CAD=FV=XmUrF3nCZF4dDom5RrWrdVe-iJocenU3cJEDx-gGkDRA@mail.gmail.com>
Hi Douglas,
On Mon, May 18, 2020 at 11:05:58AM -0700, Doug Anderson wrote:
> Sam,
>
> On Mon, May 18, 2020 at 10:59 AM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > Hi Douglas.
> >
> > > > Given the previous feedback from Linus W, Stephen, and Laurent I
> > > > expect things are good enough to land now, but it'd be good to get
> > > > confirmation (I removed some of the previous tags just to get
> > > > confirmation). If we can get review tags early next week maybe it'll
> > > > still be in time to land for 5.8?
> > >
> > > I think all the others have reviews now. Is there anything blocking
> > > them from getting applied?
> > Applied, including the small fix pointed out by Linus.
>
> Thanks! Ugh, I just realized what the problem was. I posted a v6
> with this fix but insanely somehow didn't CC you (!!!). It was here:
>
> https://lore.kernel.org/r/20200513215902.261547-1-dianders@chromium.org
>
> I'm super sorry about that and thanks for fixing the nit. That was
> the only difference between v5 and v6. I just checked what you pushed
> and it looks great, thank you.
No worries as long as what hits drm-misc-next is OK, which it was in
this case.
Sam
^ permalink raw reply
* Re: [PATCH v6 1/2] dt-bindings: mtd: Add Nand Flash Controller support for Intel LGM SoC
From: Rob Herring @ 2020-05-18 18:27 UTC (permalink / raw)
To: Ramuthevar,Vadivel MuruganX
Cc: Mason Yang, MTD Maling List, Richard Weinberger, Boris Brezillon,
Brendan Higgins, Thomas Gleixner, Miquèl Raynal, Vignesh R,
hauke.mehrtens, devicetree, Anders Roxell, Arnd Bergmann,
Andy Shevchenko, open list:MIPS, cheol.yong.kim,
linux-kernel@vger.kernel.org, qi-ming.wu
In-Reply-To: <dc51e6af-bda8-d8b9-1782-f5c4d5d3fed7@linux.intel.com>
On Thu, May 14, 2020 at 8:08 PM Ramuthevar, Vadivel MuruganX
<vadivel.muruganx.ramuthevar@linux.intel.com> wrote:
>
> Hi Rob,
>
> On 14/5/2020 8:57 pm, Rob Herring wrote:
> > On Wed, 13 May 2020 18:46:14 +0800, Ramuthevar,Vadivel MuruganX wrote:
> >> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> >>
> >> Add YAML file for dt-bindings to support NAND Flash Controller
> >> on Intel's Lightning Mountain SoC.
> >>
> >> Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> >> ---
> >> .../devicetree/bindings/mtd/intel,lgm-nand.yaml | 83 ++++++++++++++++++++++
> >> 1 file changed, 83 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
> >>
> >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/intel,lgm-nand.example.dt.yaml: nand-controller@e0f00000: 'dmas' is a dependency of 'dma-names'
> >
> > See https://patchwork.ozlabs.org/patch/1289160
> >
> > If you already ran 'make dt_binding_check' and didn't see the above
> > error(s), then make sure dt-schema is up to date:
> >
> > pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
> >
> > Please check and re-submit.
> Thank you very much for review comments...
> I didn't find build errors, successfully built.
You need to build without DT_SCHEMA_FILES set or be on 5.7-rc (you
should be on a current -rcX at least for any patch submission). This
comes from the core schema.
Rob
^ permalink raw reply
* [PATCH 1/3] dt-bindings: vendor-prefixes: Add MikroTik
From: Jonathan McDowell @ 2020-05-18 18:09 UTC (permalink / raw)
To: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <cover.1589824955.git.noodles@earth.li>
MikroTik (SIA Mikrotīkls) is a Latvian company who develop routers and
wireless ISP systems.
Signed-off-by: Jonathan McDowell <noodles@earth.li>
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index d3891386d671..d3277fe6640b 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -633,6 +633,8 @@ patternProperties:
description: Microsoft Corporation
"^mikroe,.*":
description: MikroElektronika d.o.o.
+ "^mikrotik,.*":
+ description: MikroTik
"^miniand,.*":
description: Miniand Tech
"^minix,.*":
--
2.20.1
^ permalink raw reply related
* [PATCH 2/3] ARM: dts: qcom: add ethernet definitions to ipq8064
From: Jonathan McDowell @ 2020-05-18 18:09 UTC (permalink / raw)
To: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <cover.1589824955.git.noodles@earth.li>
Plumb in the 4 IPQ8064 stmmac based ethernet interfaces.
Signed-off-by: Jonathan McDowell <noodles@earth.li>
---
arch/arm/boot/dts/qcom-ipq8064.dtsi | 108 ++++++++++++++++++++++++++++
1 file changed, 108 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index b912da9a3ff3..9c0d5510c805 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -597,6 +597,114 @@
perst-gpio = <&qcom_pinmux 63 GPIO_ACTIVE_LOW>;
};
+ nss_common: syscon@03000000 {
+ compatible = "syscon";
+ reg = <0x03000000 0x0000FFFF>;
+ };
+
+ qsgmii_csr: syscon@1bb00000 {
+ compatible = "syscon";
+ reg = <0x1bb00000 0x000001FF>;
+ };
+
+ stmmac_axi_setup: stmmac-axi-config {
+ snps,wr_osr_lmt = <7>;
+ snps,rd_osr_lmt = <7>;
+ snps,blen = <16 0 0 0 0 0 0>;
+ };
+
+ gmac0: ethernet@37000000 {
+ device_type = "network";
+ compatible = "qcom,ipq806x-gmac";
+ reg = <0x37000000 0x200000>;
+ interrupts = <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+
+ snps,axi-config = <&stmmac_axi_setup>;
+ snps,pbl = <32>;
+ snps,aal = <1>;
+
+ qcom,nss-common = <&nss_common>;
+ qcom,qsgmii-csr = <&qsgmii_csr>;
+
+ clocks = <&gcc GMAC_CORE1_CLK>;
+ clock-names = "stmmaceth";
+
+ resets = <&gcc GMAC_CORE1_RESET>;
+ reset-names = "stmmaceth";
+
+ status = "disabled";
+ };
+
+ gmac1: ethernet@37200000 {
+ device_type = "network";
+ compatible = "qcom,ipq806x-gmac";
+ reg = <0x37200000 0x200000>;
+ interrupts = <GIC_SPI 223 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+
+ snps,axi-config = <&stmmac_axi_setup>;
+ snps,pbl = <32>;
+ snps,aal = <1>;
+
+ qcom,nss-common = <&nss_common>;
+ qcom,qsgmii-csr = <&qsgmii_csr>;
+
+ clocks = <&gcc GMAC_CORE2_CLK>;
+ clock-names = "stmmaceth";
+
+ resets = <&gcc GMAC_CORE2_RESET>;
+ reset-names = "stmmaceth";
+
+ status = "disabled";
+ };
+
+ gmac2: ethernet@37400000 {
+ device_type = "network";
+ compatible = "qcom,ipq806x-gmac";
+ reg = <0x37400000 0x200000>;
+ interrupts = <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+
+ snps,axi-config = <&stmmac_axi_setup>;
+ snps,pbl = <32>;
+ snps,aal = <1>;
+
+ qcom,nss-common = <&nss_common>;
+ qcom,qsgmii-csr = <&qsgmii_csr>;
+
+ clocks = <&gcc GMAC_CORE3_CLK>;
+ clock-names = "stmmaceth";
+
+ resets = <&gcc GMAC_CORE3_RESET>;
+ reset-names = "stmmaceth";
+
+ status = "disabled";
+ };
+
+ gmac3: ethernet@37600000 {
+ device_type = "network";
+ compatible = "qcom,ipq806x-gmac";
+ reg = <0x37600000 0x200000>;
+ interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+
+ snps,axi-config = <&stmmac_axi_setup>;
+ snps,pbl = <32>;
+ snps,aal = <1>;
+
+ qcom,nss-common = <&nss_common>;
+ qcom,qsgmii-csr = <&qsgmii_csr>;
+
+ clocks = <&gcc GMAC_CORE4_CLK>;
+ clock-names = "stmmaceth";
+
+ resets = <&gcc GMAC_CORE4_RESET>;
+ reset-names = "stmmaceth";
+
+ status = "disabled";
+ };
+
vsdcc_fixed: vsdcc-regulator {
compatible = "regulator-fixed";
regulator-name = "SDCC Power";
--
2.20.1
^ permalink raw reply related
* [PATCH 0/3] Add initial support for MikroTik RB3011
From: Jonathan McDowell @ 2020-05-18 18:09 UTC (permalink / raw)
To: linux-arm-msm, devicetree, linux-kernel
This patches do some prep (device tree vendor addition, missing ethernet
definitions for IPQ8064 dts) and then add an initial device tree for the
MikroTik RB3011 1U router, which is an IPQ8064 device with 1G RAM. They
are sufficient to boot with an initrd, the serial console and the first
5 switch ports fully functional (the second bank of 5 require some extra
patches to the QCA8K driver which will follow later). The bootloader NOR
is also supported, but the 128MB NAND is not yet enabled.
Jonathan McDowell (3):
dt-bindings: vendor-prefixes: Add MikroTik
ARM: dts: qcom: add ethernet definitions to ipq8064
ARM: dts: qcom: Add MikroTik RB3011
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/qcom-ipq8064-rb3011.dts | 308 ++++++++++++++++++
arch/arm/boot/dts/qcom-ipq8064.dtsi | 108 ++++++
4 files changed, 419 insertions(+)
create mode 100644 arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
--
2.20.1
^ permalink raw reply
* Re: [PATCH v6 1/2] dt-bindings: mtd: Add Nand Flash Controller support for Intel LGM SoC
From: Rob Herring @ 2020-05-18 18:24 UTC (permalink / raw)
To: Ramuthevar,Vadivel MuruganX
Cc: linux-kernel@vger.kernel.org, MTD Maling List, devicetree,
Miquèl Raynal, Richard Weinberger, Vignesh R, Arnd Bergmann,
Brendan Higgins, Thomas Gleixner, Boris Brezillon, Anders Roxell,
Mason Yang, open list:MIPS, hauke.mehrtens, Andy Shevchenko,
qi-ming.wu, cheol.yong.kim
In-Reply-To: <3248d380-1d08-3088-8d18-0373a8a5aef9@linux.intel.com>
On Thu, May 14, 2020 at 8:06 PM Ramuthevar, Vadivel MuruganX
<vadivel.muruganx.ramuthevar@linux.intel.com> wrote:
>
> Hi Rob,
>
> Thank you for the review comments...
>
> On 14/5/2020 9:03 pm, Rob Herring wrote:
> > On Wed, May 13, 2020 at 06:46:14PM +0800, Ramuthevar,Vadivel MuruganX wrote:
> >> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> >>
> >> Add YAML file for dt-bindings to support NAND Flash Controller
> >> on Intel's Lightning Mountain SoC.
> >>
> >> Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
> >> ---
> >> .../devicetree/bindings/mtd/intel,lgm-nand.yaml | 83 ++++++++++++++++++++++
> >> 1 file changed, 83 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
> >> + clocks = <&cgu0 125>;
> >> + dma-names = "tx", "rx";
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + #clock-cells = <1>;
> >
> > This is a clock provider too?
> Yes, it is getting clock from CGU for nand-controller.
That is a clock client (e.g. 'clocks' property), not a provider. The
CGU is the provider.
Rob
^ permalink raw reply
* Re: [PATCH v2 1/6] dt-bindings: dma: dw: Convert DW DMAC to DT binding
From: Rob Herring @ 2020-05-18 17:50 UTC (permalink / raw)
To: Serge Semin
Cc: Vinod Koul, Viresh Kumar, Andy Shevchenko, Serge Semin,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Dan Williams, linux-mips, dmaengine, devicetree,
linux-kernel
In-Reply-To: <20200508105304.14065-2-Sergey.Semin@baikalelectronics.ru>
On Fri, May 08, 2020 at 01:52:59PM +0300, Serge Semin wrote:
> Modern device tree bindings are supposed to be created as YAML-files
> in accordance with dt-schema. This commit replaces the Synopsis
> Designware DMA controller legacy bare text bindings with YAML file.
> The only required prorties are "compatible", "reg", "#dma-cells" and
> "interrupts", which will be used by the driver to correctly find the
> controller memory region and handle its events. The rest of the properties
> are optional, since in case if either "dma-channels" or "dma-masters" isn't
> specified, the driver will attempt to auto-detect the IP core
> configuration.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: linux-mips@vger.kernel.org
>
> ---
>
> Changelog v2:
> - Rearrange SoBs.
> - Move $ref to the root level of the properties. So do do with the
> constraints.
> - Discard default settings defined out of the property enum constraint.
> - Replace "additionalProperties: false" with "unevaluatedProperties: false"
> property.
> - Remove a label definition from the binding example.
> ---
> .../bindings/dma/snps,dma-spear1340.yaml | 161 ++++++++++++++++++
> .../devicetree/bindings/dma/snps-dma.txt | 69 --------
> 2 files changed, 161 insertions(+), 69 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/dma/snps,dma-spear1340.yaml
> delete mode 100644 Documentation/devicetree/bindings/dma/snps-dma.txt
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v11 13/13] dt-bindings: Add media properties
From: Rob Herring @ 2020-05-18 18:21 UTC (permalink / raw)
To: Jacopo Mondi
Cc: open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB), libcamera-devel,
Mauro Carvalho Chehab, Hans Verkuil, Sakari Ailus,
Laurent Pinchart, tfiga, pavel, devicetree
In-Reply-To: <20200509090456.3496481-14-jacopo@jmondi.org>
On Sat, May 09, 2020 at 11:04:56AM +0200, Jacopo Mondi wrote:
> Add a DT header file to contain definitions for standard media properties.
>
> The file is named after:
> Documentation/devicetree/bindings/media/video-interfaces.txt
> which contains the standard media properties definitions.
>
> Initially add three macros to define the supported 'orientation'
> property values.
>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>
> I currently don't have users in mainline for this, I understand this implies
> this is probably not going to be accepted. At the same time we don't have a
> common place for media-related definitions, which support properties defined in
> video-interfaces.txt
Fine for me with no users if values already defined in binding.
>
> I leave it here at the end of the series for discussions, but I'm fine dropping
> it from the series.
>
> Thanks
> j
>
> ---
> include/dt-bindings/media/video-interfaces.h | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
> create mode 100644 include/dt-bindings/media/video-interfaces.h
>
> diff --git a/include/dt-bindings/media/video-interfaces.h b/include/dt-bindings/media/video-interfaces.h
> new file mode 100644
> index 0000000000000..404c697d6bd6e
> --- /dev/null
> +++ b/include/dt-bindings/media/video-interfaces.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
Dual license please.
> +/*
> + * include/dt-bindings/media/video-interfaces.h
> + *
> + * Copyright (C) 2020 Jacopo Mondi <jacopo@jmondi.org>
> + */
> +
> +#ifndef __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__
> +#define __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__
> +
> +#define FRONT_CAMERA <0>
> +#define BACK_CAMERA <1>
> +#define EXTERNAL_CAMERA <2>
> +
> +#endif /* __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ */
> --
> 2.26.1
>
^ permalink raw reply
* Re: [RESEND PATCH v2 2/3] dt-bindings: phy: Allow BSD licensing of marvell,mmp3-hsic-phy.yaml
From: Rob Herring @ 2020-05-18 18:18 UTC (permalink / raw)
To: Lubomir Rintel; +Cc: devicetree, linux-kernel, Kishon Vijay Abraham I
In-Reply-To: <20200509081754.474787-3-lkundrak@v3.sk>
On Sat, 9 May 2020 10:17:53 +0200, Lubomir Rintel wrote:
> I wrote this binding and I'm fine with it being GPL + BSD dual-licensed,
> as is recommended for new DT bindings.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
> .../devicetree/bindings/phy/marvell,mmp3-hsic-phy.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [RESEND PATCH v2 1/3] dt-bindings: phy: Drop reset-gpios from marvell,mmp3-hsic-phy
From: Rob Herring @ 2020-05-18 18:17 UTC (permalink / raw)
To: Lubomir Rintel; +Cc: linux-kernel, Kishon Vijay Abraham I, devicetree
In-Reply-To: <20200509081754.474787-2-lkundrak@v3.sk>
On Sat, 9 May 2020 10:17:52 +0200, Lubomir Rintel wrote:
> This has been added in error -- the PHY block doesn't have a reset pin.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
> .../devicetree/bindings/phy/marvell,mmp3-hsic-phy.yaml | 7 -------
> 1 file changed, 7 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox