* Re: [PATCH v9 02/15] clk: mux: Split out register accessors for reuse
From: Bjorn Andersson @ 2018-05-24 16:50 UTC (permalink / raw)
To: Sricharan R
Cc: robh, viresh.kumar, mark.rutland, mturquette, sboyd, linux,
andy.gross, david.brown, rjw, linux-arm-kernel, devicetree,
linux-kernel, linux-clk, linux-arm-msm, linux-soc, linux-pm,
linux
In-Reply-To: <1520347148-27852-3-git-send-email-sricharan@codeaurora.org>
On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:
> From: Stephen Boyd <sboyd@codeaurora.org>
>
> We want to reuse the logic in clk-mux.c for other clock drivers
> that don't use readl as register accessors. Fortunately, there
> really isn't much to the mux code besides the table indirection
> and quirk flags if you assume any bit shifting and masking has
> been done already. Pull that logic out into reusable functions
> that operate on an optional table and some flags so that other
> drivers can use the same logic.
>
> [Sricharan: Rebased for mainline]
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
This should read as a log, where the first entry is Stephen stating that
he acquired or wrote the code and can release it according to the
license requirements. Then you state that you acquired it, changed it
and are releasing it according to the license requirements.
PS. Please expand your last name.
Regards,
Bjorn
> ---
> drivers/clk/clk-mux.c | 74 +++++++++++++++++++++++++++----------------
> drivers/clk/nxp/clk-lpc32xx.c | 21 +++---------
> include/linux/clk-provider.h | 6 ++++
> 3 files changed, 57 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 39cabe1..28223fa 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -26,35 +26,25 @@
> * parent - parent is adjustable through clk_set_parent
> */
>
> -static u8 clk_mux_get_parent(struct clk_hw *hw)
> +unsigned int clk_mux_get_parent(struct clk_hw *hw, unsigned int val,
> + unsigned int *table,
> + unsigned long flags)
> {
> - struct clk_mux *mux = to_clk_mux(hw);
> int num_parents = clk_hw_get_num_parents(hw);
> - u32 val;
> -
> - /*
> - * FIXME need a mux-specific flag to determine if val is bitwise or numeric
> - * e.g. sys_clkin_ck's clksel field is 3 bits wide, but ranges from 0x1
> - * to 0x7 (index starts at one)
> - * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
> - * val = 0x4 really means "bit 2, index starts at bit 0"
> - */
> - val = clk_readl(mux->reg) >> mux->shift;
> - val &= mux->mask;
>
> - if (mux->table) {
> + if (table) {
> int i;
>
> for (i = 0; i < num_parents; i++)
> - if (mux->table[i] == val)
> + if (table[i] == val)
> return i;
> return -EINVAL;
> }
>
> - if (val && (mux->flags & CLK_MUX_INDEX_BIT))
> + if (val && (flags & CLK_MUX_INDEX_BIT))
> val = ffs(val) - 1;
>
> - if (val && (mux->flags & CLK_MUX_INDEX_ONE))
> + if (val && (flags & CLK_MUX_INDEX_ONE))
> val--;
>
> if (val >= num_parents)
> @@ -62,23 +52,53 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
>
> return val;
> }
> +EXPORT_SYMBOL_GPL(clk_mux_get_parent);
>
> -static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> +static u8 _clk_mux_get_parent(struct clk_hw *hw)
> {
> struct clk_mux *mux = to_clk_mux(hw);
> u32 val;
> - unsigned long flags = 0;
>
> - if (mux->table) {
> - index = mux->table[index];
> + /*
> + * FIXME need a mux-specific flag to determine if val is bitwise or
> + * numeric e.g. sys_clkin_ck's clksel field is 3 bits wide,
> + * but ranges from 0x1 to 0x7 (index starts at one)
> + * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
> + * val = 0x4 really means "bit 2, index starts at bit 0"
> + */
> + val = clk_readl(mux->reg) >> mux->shift;
> + val &= mux->mask;
> +
> + return clk_mux_get_parent(hw, val, mux->table, mux->flags);
> +}
> +
> +unsigned int clk_mux_reindex(u8 index, unsigned int *table,
> + unsigned long flags)
> +{
> + unsigned int val = index;
> +
> + if (table) {
> + val = table[val];
> } else {
> - if (mux->flags & CLK_MUX_INDEX_BIT)
> - index = 1 << index;
> + if (flags & CLK_MUX_INDEX_BIT)
> + val = 1 << index;
>
> - if (mux->flags & CLK_MUX_INDEX_ONE)
> - index++;
> + if (flags & CLK_MUX_INDEX_ONE)
> + val++;
> }
>
> + return val;
> +}
> +EXPORT_SYMBOL_GPL(clk_mux_reindex);
> +
> +static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> + struct clk_mux *mux = to_clk_mux(hw);
> + u32 val;
> + unsigned long flags = 0;
> +
> + index = clk_mux_reindex(index, mux->table, mux->flags);
> +
> if (mux->lock)
> spin_lock_irqsave(mux->lock, flags);
> else
> @@ -102,14 +122,14 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> }
>
> const struct clk_ops clk_mux_ops = {
> - .get_parent = clk_mux_get_parent,
> + .get_parent = _clk_mux_get_parent,
> .set_parent = clk_mux_set_parent,
> .determine_rate = __clk_mux_determine_rate,
> };
> EXPORT_SYMBOL_GPL(clk_mux_ops);
>
> const struct clk_ops clk_mux_ro_ops = {
> - .get_parent = clk_mux_get_parent,
> + .get_parent = _clk_mux_get_parent,
> };
> EXPORT_SYMBOL_GPL(clk_mux_ro_ops);
>
> diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
> index f5d815f..9b34150 100644
> --- a/drivers/clk/nxp/clk-lpc32xx.c
> +++ b/drivers/clk/nxp/clk-lpc32xx.c
> @@ -999,29 +999,16 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
> .set_rate = clk_divider_set_rate,
> };
>
> -static u8 clk_mux_get_parent(struct clk_hw *hw)
> +static u8 _clk_mux_get_parent(struct clk_hw *hw)
> {
> struct lpc32xx_clk_mux *mux = to_lpc32xx_mux(hw);
> - u32 num_parents = clk_hw_get_num_parents(hw);
> u32 val;
>
> regmap_read(clk_regmap, mux->reg, &val);
> val >>= mux->shift;
> val &= mux->mask;
>
> - if (mux->table) {
> - u32 i;
> -
> - for (i = 0; i < num_parents; i++)
> - if (mux->table[i] == val)
> - return i;
> - return -EINVAL;
> - }
> -
> - if (val >= num_parents)
> - return -EINVAL;
> -
> - return val;
> + return clk_mux_get_parent(hw, val, mux->table, 0);
> }
>
> static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> @@ -1036,11 +1023,11 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
> }
>
> static const struct clk_ops lpc32xx_clk_mux_ro_ops = {
> - .get_parent = clk_mux_get_parent,
> + .get_parent = _clk_mux_get_parent,
> };
>
> static const struct clk_ops lpc32xx_clk_mux_ops = {
> - .get_parent = clk_mux_get_parent,
> + .get_parent = _clk_mux_get_parent,
> .set_parent = clk_mux_set_parent,
> .determine_rate = __clk_mux_determine_rate,
> };
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index f711be6..344ad92 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -488,6 +488,12 @@ struct clk_mux {
> extern const struct clk_ops clk_mux_ops;
> extern const struct clk_ops clk_mux_ro_ops;
>
> +unsigned int clk_mux_get_parent(struct clk_hw *hw, unsigned int val,
> + unsigned int *table,
> + unsigned long flags);
> +unsigned int clk_mux_reindex(u8 index, unsigned int *table,
> + unsigned long flags);
> +
> struct clk *clk_register_mux(struct device *dev, const char *name,
> const char * const *parent_names, u8 num_parents,
> unsigned long flags,
> --
> 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: [PATCHv5 1/8] dt-bindings, firmware: add Intel Stratix10 service layer binding
From: Moritz Fischer @ 2018-05-24 17:04 UTC (permalink / raw)
To: richard.gong
Cc: catalin.marinas, will.deacon, dinguyen, robh+dt, mark.rutland,
atull, mdf, arnd, gregkh, corbet, linux-arm-kernel, linux-kernel,
devicetree, linux-fpga, linux-doc, yves.vandervennet,
richard.gong
In-Reply-To: <1527179600-26441-2-git-send-email-richard.gong@linux.intel.com>
On Thu, May 24, 2018 at 11:33:13AM -0500, richard.gong@linux.intel.com wrote:
> From: Richard Gong <richard.gong@intel.com>
>
> Add a device tree binding for the Intel Stratix10 service layer driver
>
> Signed-off-by: Richard Gong <richard.gong@intel.com>
> Signed-off-by: Alan Tull <atull@kernel.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
> v2: Change to put service layer driver node under the firmware node
> Change compatible to "intel, stratix10-svc"
> v3: No change
> v4: Add Rob's Reviewed-by
> v5: No change
> ---
> .../bindings/firmware/intel,stratix10-svc.txt | 57 ++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/firmware/intel,stratix10-svc.txt
>
> diff --git a/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.txt b/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.txt
> new file mode 100644
> index 0000000..1fa6606
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/intel,stratix10-svc.txt
> @@ -0,0 +1,57 @@
> +Intel Service Layer Driver for Stratix10 SoC
> +============================================
> +Intel Stratix10 SoC is composed of a 64 bit quad-core ARM Cortex A53 hard
> +processor system (HPS) and Secure Device Manager (SDM). When the FPGA is
> +configured from HPS, there needs to be a way for HPS to notify SDM the
> +location and size of the configuration data. Then SDM will get the
> +configuration data from that location and perform the FPGA configuration.
> +
> +To meet the whole system security needs and support virtual machine requesting
> +communication with SDM, only the secure world of software (EL3, Exception
> +Layer 3) can interface with SDM. All software entities running on other
> +exception layers must channel through the EL3 software whenever it needs
> +service from SDM.
> +
> +Intel Stratix10 service layer driver, running at privileged exception level
> +(EL1, Exception Layer 1), interfaces with the service providers and provides
> +the services for FPGA configuration, QSPI, Crypto and warm reset. Service layer
> +driver also manages secure monitor call (SMC) to communicate with secure monitor
> +code running in EL3.
> +
> +Required properties:
> +-------------------
> +The svc node has the following mandatory properties, must be located under
> +the firmware node.
> +
> +- compatible: "intel,stratix10-svc"
> +- method: smc or hvc
> + smc - Secure Monitor Call
> + hvc - Hypervisor Call
> +- memory-region:
> + phandle to the reserved memory node. See
> + Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> + for details
> +
> +Example:
> +-------
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + service_reserved: svcbuffer@0 {
> + compatible = "shared-dma-pool";
> + reg = <0x0 0x0 0x0 0x1000000>;
> + alignment = <0x1000>;
> + no-map;
> + };
> + };
> +
> + firmware {
> + svc {
> + compatible = "intel,stratix10-svc";
> + method = "smc";
> + memory-region = <&service_reserved>;
> + };
> + };
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCHv5 2/8] arm64: dts: stratix10: add stratix10 service driver binding to base dtsi
From: Moritz Fischer @ 2018-05-24 17:04 UTC (permalink / raw)
To: richard.gong
Cc: catalin.marinas, will.deacon, dinguyen, robh+dt, mark.rutland,
atull, mdf, arnd, gregkh, corbet, linux-arm-kernel, linux-kernel,
devicetree, linux-fpga, linux-doc, yves.vandervennet,
richard.gong
In-Reply-To: <1527179600-26441-3-git-send-email-richard.gong@linux.intel.com>
Hi Richard,
On Thu, May 24, 2018 at 11:33:14AM -0500, richard.gong@linux.intel.com wrote:
> From: Richard Gong <richard.gong@intel.com>
>
> Add Intel Stratix10 service layer to the device tree
>
> Signed-off-by: Richard Gong <richard.gong@intel.com>
> Signed-off-by: Alan Tull <atull@kernel.org>
Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
> v2: Change to put service layer driver node under the firmware node
> Change compatible to "intel, stratix10-svc"
> v3: No change
> v4: s/service driver/stratix10 service driver/ in subject line
> v5: No change
> ---
> arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> index d8c94d5..c257287 100644
> --- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> @@ -24,6 +24,19 @@
> #address-cells = <2>;
> #size-cells = <2>;
>
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + service_reserved: svcbuffer@0 {
> + compatible = "shared-dma-pool";
> + reg = <0x0 0x0 0x0 0x1000000>;
> + alignment = <0x1000>;
> + no-map;
> + };
> + };
> +
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -487,5 +500,13 @@
>
> status = "disabled";
> };
> +
> + firmware {
> + svc {
> + compatible = "intel,stratix10-svc";
> + method = "smc";
> + memory-region = <&service_reserved>;
> + };
> + };
> };
> };
> --
> 2.7.4
>
^ permalink raw reply
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers
From: Jae Hyun Yoo @ 2018-05-24 17:09 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree,
linux-kernel@vger.kernel.org, Andrew Jeffery, Arnd Bergmann,
Jason M Biils, Joel Stanley
In-Reply-To: <CAL_JsqK0HRi1meN=Y8jxRHrvrBwH_bZj3WVcYwy9MS0ZBJt0gw@mail.gmail.com>
On 5/24/2018 6:47 AM, Rob Herring wrote:
> On Wed, May 23, 2018 at 4:56 PM, Jae Hyun Yoo
> <jae.hyun.yoo@linux.intel.com> wrote:
>> On 5/23/2018 1:03 PM, Jae Hyun Yoo wrote:
>>>
>>> On 5/23/2018 12:33 PM, Rob Herring wrote:
>>>>
>>>> On Wed, May 23, 2018 at 11:37 AM, Jae Hyun Yoo
>>>> <jae.hyun.yoo@linux.intel.com> wrote:
>>>>>
>>>>> On 5/23/2018 8:11 AM, Rob Herring wrote:
>>>>>>
>>>>>>
>>>>>> On Tue, May 22, 2018 at 12:18 PM, Jae Hyun Yoo
>>>>>> <jae.hyun.yoo@linux.intel.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 5/22/2018 9:42 AM, Rob Herring wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This commit adds dt-bindings documents for PECI hwmon client
>>>>>>>>> drivers.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>>>>>>>>> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com>
>>>>>>>>> Reviewed-by: James Feist <james.feist@linux.intel.com>
>>>>>>>>> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com>
>>>>>>>>> Cc: Andrew Jeffery <andrew@aj.id.au>
>>>>>>>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>>>>>>>> Cc: Jason M Biils <jason.m.bills@linux.intel.com>
>>>>>>>>> Cc: Joel Stanley <joel@jms.id.au>
>>>>>>>>> ---
>>>>>>>>> .../bindings/hwmon/peci-cputemp.txt | 23
>>>>>>>>> ++++++++++++++++++
>>>>>>>>> .../bindings/hwmon/peci-dimmtemp.txt | 24
>>>>>>>>> +++++++++++++++++++
>>>>>>>>> 2 files changed, 47 insertions(+)
>>>>>>>>> create mode 100644
>>>>>>>>> Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>>>>>> create mode 100644
>>>>>>>>> Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt
>>>>>>>>>
>>>>>>>>> diff --git
>>>>>>>>> a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>>>>>> b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>>>>>> new file mode 100644
>>>>>>>>> index 000000000000..2f59aee12d9e
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>>>>>> @@ -0,0 +1,23 @@
>>>>>>>>> +Bindings for Intel PECI (Platform Environment Control Interface)
>>>>>>>>> cputemp
>>>>>>>>> driver.
>>>>>>>>> +
>>>>>>>>> +Required properties:
>>>>>>>>> +- compatible : Should be "intel,peci-cputemp".
>>>>>>>>> +- reg : Should contain address of a client CPU. Address
>>>>>>>>> range
>>>>>>>>> of
>>>>>>>>> CPU
>>>>>>>>> + clients is starting from 0x30 based on PECI
>>>>>>>>> specification.
>>>>>>>>> +
>>>>>>>>> +Example:
>>>>>>>>> + peci-bus@0 {
>>>>>>>>> + #address-cells = <1>;
>>>>>>>>> + #size-cells = <0>;
>>>>>>>>> + < more properties >
>>>>>>>>> +
>>>>>>>>> + peci-cputemp@30 {
>>>>>>>>> + compatible = "intel,peci-cputemp";
>>>>>>>>> + reg = <0x30>;
>>>>>>>>> + };
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> [...]
>>>>>>>>
>>>>>>>>> + peci-dimmtemp@30 {
>>>>>>>>> + compatible = "intel,peci-dimmtemp";
>>>>>>>>> + reg = <0x30>;
>>>>>>>>> + };
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> As I said in the prior version, 2 nodes at the same address is wrong.
>>>>>>>>
>>>>>>>> Rob
>>>>>>>>
>>>>>>>
>>>>>>> In PECI bus, there is one and only bus host (adapter) and multiple
>>>>>>> clients on a PECI bus, and PECI spec doesn't allow multiple
>>>>>>> originators
>>>>>>> so only the host device can originate message.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Yes, I get that. A single host still has to address slave devices.
>>>>>>
>>>>>>> In this implementation,
>>>>>>> all message transactions on a bus from client driver modules and user
>>>>>>> space will be serialized well in the PECI core bus driver so bus
>>>>>>> occupation and traffic arbitration will be managed well in the PECI
>>>>>>> core
>>>>>>> bus driver even in case of a bus has 2 client drivers at the same
>>>>>>> address. I'm sure that this implementation doesn't make that kind of
>>>>>>> problem to OS.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Multiple clients to a single device is common, but that is a software
>>>>>> problem and doesn't belong in DT.
>>>>>>
>>>>>> I don't think there is a single other case in the kernel where
>>>>>> multiple drivers can bind to the same device at a given bus address.
>>>>>> That is why we have things like MFD. Though in this case, why can't
>>>>>> one hwmon driver register multiple hwmon devices (cpu and dimm temps)?
>>>>>>
>>>>>
>>>>> It was implemented as a single driver until v2 but dimm temps need
>>>>> delayed creation unlikely the cpu temps on hwmon subsystem because of
>>>>> memory training behavior of remote x86 cpus. Since hwmon doesn't allow
>>>>> incremental creation, I had to divide it into two, cputemp and dimmtemp,
>>>>> so that cputemp can be registered immediately when the remote x86 cpu
>>>>> turns on and dimmtemp can be registered by delayed creation. It is the
>>>>> reason why I had to make the two hwmon driver modules that sharing a
>>>>> single device address.
>>>>
>>>>
>>>> That all sounds like kernel problems to me. Stop designing your DT
>>>> binding around what the kernel can or can't *currently* support.
>>>>
>>>>> Additionally, PECI isn't limited for temperature
>>>>> monitoring feature but it can be used for other functions such as
>>>>> platform management, cpu interface tuning and diagnostics and failure
>>>>> analysis, so in case of adding a new driver for the functions, we should
>>>>> add an another DT node which is sharing the same cpu address.
>>>>
>>>>
>>>> No, the driver should add support for those additional functions.
>>>> Perhaps you will need to use MFD.
>>>>
>>>
>>> Do you mean that the device address sharing is acceptable if I make
>>> these nodes under "simple-mfd"?
>>>
>>> Thanks,
>>>
>>> -Jae
>>
>>
>> Hi Rob,
>>
>> I'm planning to change the whole PECI node like below:
>>
>> peci: peci@1e78b000 {
>> compatible = "simple-bus";
>> #address-cells = <1>;
>> #size-cells = <1>;
>> ranges = <0x0 0x1e78b000 0x60>;
>>
>> peci0: peci-bus@0 {
>> compatible = "aspeed,ast2500-peci";
>> reg = <0x0 0x60>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>> interrupts = <15>;
>> clocks = <&syscon ASPEED_CLK_GATE_REFCLK>;
>> resets = <&syscon ASPEED_RESET_PECI>;
>> clock-frequency = <24000000>;
>> msg-timing = <1>;
>> addr-timing = <1>;
>> rd-sampling-point = <8>;
>> cmd-timeout-ms = <1000>;
>> status = "disabled";
>>
>> peci-client@30 {
>> compatible = "simple-mfd", "syscon";
>
> These compatibles alone is not correct. There should be a specific
> compatible for the device.
>
> Also, I don't think "syscon" even makes sense in this case.
>
Got it. I'll change it like:
compatible = "intel,peci-client", "simple-mfd";
so that drivers could be instantiated altogether if the drivers use the
"intel,peci-client" compatible string.
>> reg = <0x30>;
>>
>> cputemp: cputemp {
>> compatible = "intel,peci-cputemp";
>> };
>
> There is no point in this node being in DT. It doesn't define any
> resources. All it does is provide you a convenient way to bind your
> driver, but that is not the purpose of DT. Put a specific compatible
> in the parent and its driver can instantiate whatever child devices it
> wants.
>
My intention is making each driver can be selectively instantiated by
this node and it could use its parent reg resource which is in
simple-mfd node. If the selective instantiation is not needed, drivers
could use "intel,peci-client". Please correct me if it is still
unacceptable.
Thanks,
-Jae
^ permalink raw reply
* Re: [PATCH v3 4/6] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Guenter Roeck @ 2018-05-24 17:11 UTC (permalink / raw)
To: Ray Jui
Cc: Wim Van Sebroeck, Rob Herring, Mark Rutland, Frank Rowand,
Catalin Marinas, Will Deacon, Robin Murphy, linux-watchdog,
devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
In-Reply-To: <5e3d05f2-d526-9108-e2fd-13573458be85@broadcom.com>
On Thu, May 24, 2018 at 09:36:25AM -0700, Ray Jui wrote:
>
>
> On 5/24/2018 9:19 AM, Guenter Roeck wrote:
> >On Wed, May 23, 2018 at 05:15:22PM -0700, Ray Jui wrote:
> >>If the watchdog hardware is already enabled during the boot process,
> >>when the Linux watchdog driver loads, it should reset the watchdog and
> >>tell the watchdog framework. As a result, ping can be generated from
> >>the watchdog framework, until the userspace watchdog daemon takes over
> >>control
> >>
> >>Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> >>Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> >>Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> >
> >Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> >
> >I have one question, though: Is it really correct that both
> >INT_ENABLE _and_ RESET_ENABLE have to be set to enable the watdog ?
> >What if only RESET_ENABLE is set ?
>
> According to the SP805 TRM, INT_ENABLE needs to be set to high to enable the
> counter and the interrupt. Counter will be stopped if INT_ENABLE is cleared.
> So yes, INT_ENABLE needs to be set.
>
Excellent, thanks for the clarification.
Guenter
^ permalink raw reply
* Re: [PATCH v3 2/6] Documentation: DT: Add optional 'timeout-sec' property for sp805
From: Guenter Roeck @ 2018-05-24 17:12 UTC (permalink / raw)
To: Ray Jui
Cc: Wim Van Sebroeck, Rob Herring, Mark Rutland, Frank Rowand,
Catalin Marinas, Will Deacon, Robin Murphy, linux-watchdog,
devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
In-Reply-To: <621dc488-2d8d-003b-9bd1-4de59e8d178b@broadcom.com>
On Thu, May 24, 2018 at 09:42:20AM -0700, Ray Jui wrote:
>
>
> On 5/24/2018 9:16 AM, Guenter Roeck wrote:
> >On Wed, May 23, 2018 at 05:15:20PM -0700, Ray Jui wrote:
> >>Update the SP805 binding document to add optional 'timeout-sec'
> >>devicetree property
> >>
> >>Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> >>---
> >> Documentation/devicetree/bindings/watchdog/arm,sp805.txt | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >>diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> >>index 0fa3629..1debea3 100644
> >>--- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> >>+++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
> >>@@ -19,6 +19,8 @@ Required properties:
> >> Optional properties:
> >> - interrupts: Should specify WDT interrupt number
> >>+- timeout-sec: Should specify default WDT timeout in seconds. If unset, the
> >>+ default timeout in the driver is 30 seconds
> >
> >"... the default timeout is determined by the driver" might be better.
> >If you want to mandate a default here (not sure if that is a good idea),
> >I would suggest to use something like "should be 30 seconds".
> >
>
> Okay. This can be changed to:
>
> - timeout-sec: Should specify default WDT timeout in seconds. If unset, the
> default timeout is determined by the driver.
>
> Please advise how to proceed with this patch series. Should I make the above
> modification and send out v4?
I would suggest to wait a day or two, then send out v4 if there are no further
comments.
Thanks,
Guenter
^ permalink raw reply
* RE: [PATCH 4/9] regulator: bd71837: Devicetree bindings for BD71837 regulators
From: Vaittinen, Matti @ 2018-05-24 17:30 UTC (permalink / raw)
To: Mark Brown
Cc: mturquette@baylibre.com, sboyd@kernel.org, robh+dt@kernel.org,
mark.rutland@arm.com, lee.jones@linaro.org, lgirdwood@gmail.com,
mazziesaccount@gmail.com, linux-clk@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Mutanen, Mikko, Haikola, Heikki
In-Reply-To: <20180524140118.GS4828@sirena.org.uk>
Hello Mark,
First of all, thank you for taking your time to check the patches. I do
appreciate it. I find reading patches hard myself.
> From: Mark Brown [broonie@kernel.org]
> Sent: Thursday, May 24, 2018 5:01 PM
>
> > On Thu, May 24, 2018 at 08:57:52AM +0300, Matti Vaittinen wrote:
>
> > +Required properties:
> > + - compatible: should be "rohm,bd71837-pmic".
> > + - regulator-name: should be "buck1", ..., "buck8" and "ldo1", ..., "ldo7"
>
> The MFD is for a single device, there should be no need for compatibles
> on subfunctions.
I will check this. I must admit I am not sure what is the de-facto mechanism
for assigning the correct device-tree nodes to sub devices if compatibles
are not used? I think I saw device-tree node name being used for regulators
but how is it done for example with clk? I would be grateful if anyone could
point me to right direction with this.
Also, another thing I was wondering is how supply regulators should be
handled? In this case the LDO5 is supplied by BUCK6 and LDO6 by
BUCK7.
>From generic regulator bindings
/Documentation/devicetree/bindings/regulator/regulator.txt
I found statement:
> - <name>-supply: phandle to the parent supply/regulator node
and
> Regulator Consumers:
> Consumer nodes can reference one or more of its supplies/
> regulators using the below bindings.
>
> - <name>-supply: phandle to the regulator node
>
> These are the same bindings that a regulator in the above
> example used to reference its own supply, in which case
> ts just seen as a special case of a regulator being a
> consumer itself.
but I did not find handling for the supply properties from regulator core.
Thus I ended up hard coding the supply relation in driver. This means
that buck6 name must be fixed.
Br,
Matti Vaittinen
^ permalink raw reply
* RE: [PATCH 9/9] regulator: bd71837: Build BD71837 regulator driver
From: Vaittinen, Matti @ 2018-05-24 17:34 UTC (permalink / raw)
To: Mark Brown
Cc: mturquette@baylibre.com, sboyd@kernel.org, robh+dt@kernel.org,
mark.rutland@arm.com, lee.jones@linaro.org, lgirdwood@gmail.com,
mazziesaccount@gmail.com, linux-clk@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Mutanen, Mikko, Haikola, Heikki
In-Reply-To: <20180524140159.GT4828@sirena.org.uk>
> On Thu, May 24, 2018 at 09:01:14AM +0300, Matti Vaittinen wrote:
> > Configurations and Makefile for BD71837 regulator driver
> >
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > ---
> > drivers/regulator/Kconfig | 11 +++++++++++
> > drivers/regulator/Makefile | 1 +
> > 2 files changed, 12 insertions(+)
>
> Just squash this into the single patch adding the driver, it makes life
> easier.
Will do. I was wondering if it is nicer to have smaller patches and thus
decided to split this but if it is easier to read them in one patch
- no problem.
Br,
Matti Vaittinen
^ permalink raw reply
* Re: [PATCH v9 01/15] ARM: Add Krait L2 register accessor functions
From: Bjorn Andersson @ 2018-05-24 17:39 UTC (permalink / raw)
To: Sricharan R
Cc: robh, viresh.kumar, mark.rutland, mturquette, sboyd, linux,
andy.gross, david.brown, rjw, linux-arm-kernel, devicetree,
linux-kernel, linux-clk, linux-arm-msm, linux-soc, linux-pm,
linux
In-Reply-To: <1520347148-27852-2-git-send-email-sricharan@codeaurora.org>
On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:
> From: Stephen Boyd <sboyd@codeaurora.org>
>
> Krait CPUs have a handful of L2 cache controller registers that
> live behind a cp15 based indirection register. First you program
> the indirection register (l2cpselr) to point the L2 'window'
> register (l2cpdr) at what you want to read/write. Then you
> read/write the 'window' register to do what you want. The
> l2cpselr register is not banked per-cpu so we must lock around
> accesses to it to prevent other CPUs from re-pointing l2cpdr
> underneath us.
>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
This should have your signed-off-by here as well.
Apart from that:
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> ---
> arch/arm/common/Kconfig | 3 ++
> arch/arm/common/Makefile | 1 +
> arch/arm/common/krait-l2-accessors.c | 48 +++++++++++++++++++++++++++++++
> arch/arm/include/asm/krait-l2-accessors.h | 10 +++++++
> 4 files changed, 62 insertions(+)
> create mode 100644 arch/arm/common/krait-l2-accessors.c
> create mode 100644 arch/arm/include/asm/krait-l2-accessors.h
>
> diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
> index e5ad070..c8e1986 100644
> --- a/arch/arm/common/Kconfig
> +++ b/arch/arm/common/Kconfig
> @@ -7,6 +7,9 @@ config DMABOUNCE
> bool
> select ZONE_DMA
>
> +config KRAIT_L2_ACCESSORS
> + bool
> +
> config SHARP_LOCOMO
> bool
>
> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> index 70b4a14..eec6cd1 100644
> --- a/arch/arm/common/Makefile
> +++ b/arch/arm/common/Makefile
> @@ -7,6 +7,7 @@ obj-y += firmware.o
>
> obj-$(CONFIG_SA1111) += sa1111.o
> obj-$(CONFIG_DMABOUNCE) += dmabounce.o
> +obj-$(CONFIG_KRAIT_L2_ACCESSORS) += krait-l2-accessors.o
> obj-$(CONFIG_SHARP_LOCOMO) += locomo.o
> obj-$(CONFIG_SHARP_PARAM) += sharpsl_param.o
> obj-$(CONFIG_SHARP_SCOOP) += scoop.o
> diff --git a/arch/arm/common/krait-l2-accessors.c b/arch/arm/common/krait-l2-accessors.c
> new file mode 100644
> index 0000000..9a97dda
> --- /dev/null
> +++ b/arch/arm/common/krait-l2-accessors.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, The Linux Foundation. All rights reserved.
> +
> +#include <linux/spinlock.h>
> +#include <linux/export.h>
> +
> +#include <asm/barrier.h>
> +#include <asm/krait-l2-accessors.h>
> +
> +static DEFINE_RAW_SPINLOCK(krait_l2_lock);
> +
> +void krait_set_l2_indirect_reg(u32 addr, u32 val)
> +{
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&krait_l2_lock, flags);
> + /*
> + * Select the L2 window by poking l2cpselr, then write to the window
> + * via l2cpdr.
> + */
> + asm volatile ("mcr p15, 3, %0, c15, c0, 6 @ l2cpselr" : : "r" (addr));
> + isb();
> + asm volatile ("mcr p15, 3, %0, c15, c0, 7 @ l2cpdr" : : "r" (val));
> + isb();
> +
> + raw_spin_unlock_irqrestore(&krait_l2_lock, flags);
> +}
> +EXPORT_SYMBOL(krait_set_l2_indirect_reg);
> +
> +u32 krait_get_l2_indirect_reg(u32 addr)
> +{
> + u32 val;
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&krait_l2_lock, flags);
> + /*
> + * Select the L2 window by poking l2cpselr, then read from the window
> + * via l2cpdr.
> + */
> + asm volatile ("mcr p15, 3, %0, c15, c0, 6 @ l2cpselr" : : "r" (addr));
> + isb();
> + asm volatile ("mrc p15, 3, %0, c15, c0, 7 @ l2cpdr" : "=r" (val));
> +
> + raw_spin_unlock_irqrestore(&krait_l2_lock, flags);
> +
> + return val;
> +}
> +EXPORT_SYMBOL(krait_get_l2_indirect_reg);
> diff --git a/arch/arm/include/asm/krait-l2-accessors.h b/arch/arm/include/asm/krait-l2-accessors.h
> new file mode 100644
> index 0000000..dd7c474
> --- /dev/null
> +++ b/arch/arm/include/asm/krait-l2-accessors.h
> @@ -0,0 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018, The Linux Foundation. All rights reserved.
> +
> +#ifndef __ASMARM_KRAIT_L2_ACCESSORS_H
> +#define __ASMARM_KRAIT_L2_ACCESSORS_H
> +
> +extern void krait_set_l2_indirect_reg(u32 addr, u32 val);
> +extern u32 krait_get_l2_indirect_reg(u32 addr);
> +
> +#endif
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
^ permalink raw reply
* [PATCH] ARM: dts: pxa3xx: fix MMC clocks
From: Daniel Mack @ 2018-05-24 17:43 UTC (permalink / raw)
To: robert.jarzmik, haojian.zhuang
Cc: devicetree, robh+dt, Daniel Mack, linux-arm-kernel
The clocks for the 3 MMC controllers on pxa3xx platforms are CLK_MMC1,
CLK_MMC2 and CLK_MMC3. CLK_MMC is only for pxa2xx.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
arch/arm/boot/dts/pxa3xx.dtsi | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/pxa3xx.dtsi b/arch/arm/boot/dts/pxa3xx.dtsi
index 132cce03414f..7ffe06f1dcc0 100644
--- a/arch/arm/boot/dts/pxa3xx.dtsi
+++ b/arch/arm/boot/dts/pxa3xx.dtsi
@@ -174,7 +174,7 @@
compatible = "marvell,pxa-mmc";
reg = <0x41100000 0x1000>;
interrupts = <23>;
- clocks = <&clks CLK_MMC>;
+ clocks = <&clks CLK_MMC1>;
dmas = <&pdma 21 3
&pdma 22 3>;
dma-names = "rx", "tx";
@@ -185,7 +185,7 @@
compatible = "marvell,pxa-mmc";
reg = <0x42000000 0x1000>;
interrupts = <41>;
- clocks = <&clks CLK_MMC1>;
+ clocks = <&clks CLK_MMC2>;
dmas = <&pdma 93 3
&pdma 94 3>;
dma-names = "rx", "tx";
@@ -196,7 +196,7 @@
compatible = "marvell,pxa-mmc";
reg = <0x42500000 0x1000>;
interrupts = <55>;
- clocks = <&clks CLK_MMC2>;
+ clocks = <&clks CLK_MMC3>;
dmas = <&pdma 46 3
&pdma 47 3>;
dma-names = "rx", "tx";
--
2.14.3
^ permalink raw reply related
* [PATCH v2 0/8] Make deferring probe forever optional
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel
This series came out of a discussion on the ARM boot-architecture
list[1] about DT forwards and backwards compatibility issues. There are
issues with newer DTs breaking on older, stable kernels. Some of these
are difficult to solve, but cases of optional devices not having
kernel support should be solvable.
I tested this on a RPi3 B with the pinctrl driver forced off. With this
change, the MMC/SD and UART drivers can function without the pinctrl
driver.
v2:
- Add a DT property for pinctrl to flag using defaults
- Add a debug timeout to stop deferring some number of seconds after
initcalls are done (giving modules a chance to load)
- Split pinctrl support to its own patch
- WARN when we stop deferring probe for a device
- Add IOMMU support
- Add PM domain support
Rob
[1] https://lists.linaro.org/pipermail/boot-architecture/2018-April/000466.html
Rob Herring (8):
driver core: make deferring probe after init optional
driver core: add a deferred probe timeout
dt-bindings: pinctrl: add a 'pinctrl-use-default' property
arm: dts: bcm283x: mark the UART pin muxing nodes with
pinctrl-use-default
pinctrl: optionally stop deferring probe at end of initcalls
iommu: Stop deferring probe at end of initcalls
iommu: Remove IOMMU_OF_DECLARE
PM / Domains: Stop deferring probe at the end of initcall
.../admin-guide/kernel-parameters.txt | 7 +++
.../bindings/pinctrl/pinctrl-bindings.txt | 6 +++
arch/arm/boot/dts/bcm283x.dtsi | 2 +
drivers/base/dd.c | 43 +++++++++++++++++++
drivers/base/power/domain.c | 2 +-
drivers/iommu/arm-smmu-v3.c | 2 -
drivers/iommu/arm-smmu.c | 7 ---
drivers/iommu/exynos-iommu.c | 2 -
drivers/iommu/ipmmu-vmsa.c | 3 --
drivers/iommu/msm_iommu.c | 2 -
drivers/iommu/of_iommu.c | 21 +--------
drivers/iommu/qcom_iommu.c | 2 -
drivers/iommu/rockchip-iommu.c | 2 -
drivers/pinctrl/devicetree.c | 14 ++++--
include/linux/device.h | 2 +
include/linux/of_iommu.h | 4 --
16 files changed, 73 insertions(+), 48 deletions(-)
--
2.17.0
^ permalink raw reply
* [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
boot-architecture-cunTk1MwBs8s++Sfvej+rw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180524175024.19874-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Deferred probe will currently wait forever on dependent devices to probe,
but sometimes a driver will never exist. It's also not always critical for
a driver to exist. Platforms can rely on default configuration from the
bootloader or reset defaults for things such as pinctrl and power domains.
This is often the case with initial platform support until various drivers
get enabled. There's at least 2 scenarios where deferred probe can render
a platform broken. Both involve using a DT which has more devices and
dependencies than the kernel supports. The 1st case is a driver may be
disabled in the kernel config. The 2nd case is the kernel version may
simply not have the dependent driver. This can happen if using a newer DT
(provided by firmware perhaps) with a stable kernel version.
Subsystems or drivers may opt-in to this behavior by calling
driver_deferred_probe_check_init_done() instead of just returning
-EPROBE_DEFER. They may use additional information from DT or kernel's
config to decide whether to continue to defer probe or not.
Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/base/dd.c | 17 +++++++++++++++++
include/linux/device.h | 2 ++
2 files changed, 19 insertions(+)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index c9f54089429b..d6034718da6f 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -226,6 +226,16 @@ void device_unblock_probing(void)
driver_deferred_probe_trigger();
}
+int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
+{
+ if (optional && initcalls_done) {
+ dev_WARN(dev, "ignoring dependency for device, assuming no driver");
+ return -ENODEV;
+ }
+
+ return -EPROBE_DEFER;
+}
+
/**
* deferred_probe_initcall() - Enable probing of deferred devices
*
@@ -240,6 +250,13 @@ static int deferred_probe_initcall(void)
/* Sort as many dependencies as possible before exiting initcalls */
flush_work(&deferred_probe_work);
initcalls_done = true;
+
+ /*
+ * Trigger deferred probe again, this time we won't defer anything
+ * that is optional
+ */
+ driver_deferred_probe_trigger();
+ flush_work(&deferred_probe_work);
return 0;
}
late_initcall(deferred_probe_initcall);
diff --git a/include/linux/device.h b/include/linux/device.h
index 477956990f5e..f3dafd44c285 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -334,6 +334,8 @@ struct device *driver_find_device(struct device_driver *drv,
struct device *start, void *data,
int (*match)(struct device *dev, void *data));
+int driver_deferred_probe_check_init_done(struct device *dev, bool optional);
+
/**
* struct subsys_interface - interfaces to device functions
* @name: name of the device function
--
2.17.0
_______________________________________________
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture
^ permalink raw reply related
* [PATCH v2 2/8] driver core: add a deferred probe timeout
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel
In-Reply-To: <20180524175024.19874-1-robh@kernel.org>
Deferring probe can wait forever on dependencies that may never appear
for a variety of reasons. This can be difficult to debug especially if
the console has dependencies or userspace fails to boot to a shell. Add
a timeout to retry probing without possibly optional dependencies and to
dump out the deferred probe pending list after retrying.
This mechanism is intended for debug purposes. It won't work for the
console which needs to be enabled before userspace starts. However, if
the console's dependencies are resolved, then the kernel log will be
printed (as opposed to no output).
Signed-off-by: Rob Herring <robh@kernel.org>
---
.../admin-guide/kernel-parameters.txt | 7 +++++
drivers/base/dd.c | 28 ++++++++++++++++++-
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 11fc28ecdb6d..dd3f40b34a24 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -809,6 +809,13 @@
Defaults to the default architecture's huge page size
if not specified.
+ deferred_probe_timeout=
+ [KNL] Set a timeout in seconds for deferred probe to
+ give up waiting on dependencies to probe. Only specific
+ dependencies (subsystems or drivers) that have opted in
+ will be ignored. This option also dumps out devices
+ still on the deferred probe list after retrying.
+
dhash_entries= [KNL]
Set number of hash buckets for dentry cache.
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index d6034718da6f..4133b240c7e4 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -226,9 +226,17 @@ void device_unblock_probing(void)
driver_deferred_probe_trigger();
}
+static int deferred_probe_timeout = -1;
+static int __init deferred_probe_timeout_setup(char *str)
+{
+ deferred_probe_timeout = simple_strtol(str, NULL, 0);
+ return 1;
+}
+__setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
+
int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
{
- if (optional && initcalls_done) {
+ if ((optional || !deferred_probe_timeout) && initcalls_done) {
dev_WARN(dev, "ignoring dependency for device, assuming no driver");
return -ENODEV;
}
@@ -236,6 +244,19 @@ int driver_deferred_probe_check_init_done(struct device *dev, bool optional)
return -EPROBE_DEFER;
}
+static void deferred_probe_timeout_work_func(struct work_struct *work)
+{
+ struct device_private *private, *p;
+
+ deferred_probe_timeout = 0;
+ driver_deferred_probe_trigger();
+ flush_work(&deferred_probe_work);
+
+ list_for_each_entry_safe(private, p, &deferred_probe_pending_list, deferred_probe)
+ dev_info(private->device, "deferred probe pending");
+}
+static DECLARE_DELAYED_WORK(deferred_probe_timeout_work, deferred_probe_timeout_work_func);
+
/**
* deferred_probe_initcall() - Enable probing of deferred devices
*
@@ -257,6 +278,11 @@ static int deferred_probe_initcall(void)
*/
driver_deferred_probe_trigger();
flush_work(&deferred_probe_work);
+
+ if (deferred_probe_timeout > 0) {
+ schedule_delayed_work(&deferred_probe_timeout_work,
+ deferred_probe_timeout * HZ);
+ }
return 0;
}
late_initcall(deferred_probe_initcall);
--
2.17.0
^ permalink raw reply related
* [PATCH v2 3/8] dt-bindings: pinctrl: add a 'pinctrl-use-default' property
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel
In-Reply-To: <20180524175024.19874-1-robh@kernel.org>
Pin setup may be optional in some cases such as the reset default works
or the pin setup is done by the bootloader. In these cases, it is optional
for the OS to support managing the pin controller and pin setup. In order
to support this scenario, add a property 'pinctrl-use-default' to indicate
that the pin configuration is optional.
Signed-off-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/pinctrl/pinctrl-bindings.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
index ad9bbbba36e9..cef2b5855d60 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -103,6 +103,12 @@ Optional properties:
#pinctrl-cells: Number of pin control cells in addition to the index within the
pin controller device instance
+pinctrl-use-default: Boolean. Indicates that the OS can use the boot default
+ pin configuration. This allows using an OS that does not have a
+ driver for the pin controller. This property can be set either
+ globally for the pin controller or in child nodes for individual
+ pin group control.
+
Pin controller devices should contain the pin configuration nodes that client
devices reference.
--
2.17.0
^ permalink raw reply related
* [PATCH v2 4/8] arm: dts: bcm283x: mark the UART pin muxing nodes with pinctrl-use-default
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel
In-Reply-To: <20180524175024.19874-1-robh@kernel.org>
Signed-off-by: Rob Herring <robh@kernel.org>
---
arch/arm/boot/dts/bcm283x.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index ac00e730f898..c8b8ede3d273 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -321,6 +321,7 @@
};
uart0_gpio14: uart0_gpio14 {
+ pinctrl-use-default;
brcm,pins = <14 15>;
brcm,function = <BCM2835_FSEL_ALT0>;
};
@@ -353,6 +354,7 @@
};
uart1_gpio14: uart1_gpio14 {
+ pinctrl-use-default;
brcm,pins = <14 15>;
brcm,function = <BCM2835_FSEL_ALT5>;
};
--
2.17.0
^ permalink raw reply related
* [PATCH v2 5/8] pinctrl: optionally stop deferring probe at end of initcalls
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
boot-architecture-cunTk1MwBs8s++Sfvej+rw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180524175024.19874-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
If the pinctrl node in DT indicates that pin setup is optional and the
defaults can be used with the 'pinctrl-use-default', then only defer probe
until initcalls are done. This gives platforms the option to work without
their pinctrl driver being enabled.
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/pinctrl/devicetree.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index b601039d6c69..74a31074b406 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -110,17 +110,23 @@ static int dt_to_map_one_config(struct pinctrl *p,
int ret;
struct pinctrl_map *map;
unsigned num_maps;
+ bool pctl_optional = false;
/* Find the pin controller containing np_config */
np_pctldev = of_node_get(np_config);
for (;;) {
+ if (!pctl_optional)
+ pctl_optional = of_property_read_bool(np_pctldev, "pinctrl-use-default");
+
np_pctldev = of_get_next_parent(np_pctldev);
if (!np_pctldev || of_node_is_root(np_pctldev)) {
- dev_info(p->dev, "could not find pctldev for node %pOF, deferring probe\n",
- np_config);
of_node_put(np_pctldev);
- /* OK let's just assume this will appear later then */
- return -EPROBE_DEFER;
+ ret = driver_deferred_probe_check_init_done(p->dev, pctl_optional);
+ if (ret == -EPROBE_DEFER)
+ /* OK let's just assume this will appear later then */
+ dev_info(p->dev, "could not find pctldev for node %pOF, deferring probe\n",
+ np_config);
+ return ret;
}
/* If we're creating a hog we can use the passed pctldev */
if (pctldev && (np_pctldev == p->dev->of_node))
--
2.17.0
_______________________________________________
boot-architecture mailing list
boot-architecture@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/boot-architecture
^ permalink raw reply related
* [PATCH v2 6/8] iommu: Stop deferring probe at end of initcalls
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
boot-architecture-cunTk1MwBs8s++Sfvej+rw,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180524175024.19874-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
The IOMMU subsystem has its own mechanism to not defer probe if driver
support is missing. Now that the driver core supports stopping deferring
probe if drivers aren't built-in (and probed), use the driver core
support so the IOMMU specific support can be removed.
Cc: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/iommu/of_iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5c36a8b7656a..2aac8387717c 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -133,7 +133,7 @@ static int of_iommu_xlate(struct device *dev,
* a proper probe-ordering dependency mechanism in future.
*/
if (!ops)
- return -EPROBE_DEFER;
+ return driver_deferred_probe_check_init_done(dev, true);
return ops->of_xlate(dev, iommu_spec);
}
--
2.17.0
^ permalink raw reply related
* [PATCH v2 7/8] iommu: Remove IOMMU_OF_DECLARE
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
boot-architecture-cunTk1MwBs8s++Sfvej+rw,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Will Deacon,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Krzysztof Kozlowski,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kukjin Kim,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20180524175024.19874-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Now that we use the driver core to stop deferred probe for missing
drivers, IOMMU_OF_DECLARE can be removed.
This is slightly less optimal than having a list of built-in drivers in
that we'll now defer probe twice before giving up. This shouldn't have a
significant impact on boot times as past discussions about deferred
probe have given no evidence of deferred probe having a substantial
impact.
Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
Cc: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
Cc: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
Cc: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Kukjin Kim <kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Rob Clark <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
drivers/iommu/arm-smmu-v3.c | 2 --
drivers/iommu/arm-smmu.c | 7 -------
drivers/iommu/exynos-iommu.c | 2 --
drivers/iommu/ipmmu-vmsa.c | 3 ---
drivers/iommu/msm_iommu.c | 2 --
drivers/iommu/of_iommu.c | 19 +------------------
drivers/iommu/qcom_iommu.c | 2 --
drivers/iommu/rockchip-iommu.c | 2 --
include/linux/of_iommu.h | 4 ----
9 files changed, 1 insertion(+), 42 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 1d647104bccc..22bdabd3d8e0 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2915,8 +2915,6 @@ static struct platform_driver arm_smmu_driver = {
};
module_platform_driver(arm_smmu_driver);
-IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3");
-
MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 69e7c60792a8..9dd7cbaa3b0c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2211,13 +2211,6 @@ static struct platform_driver arm_smmu_driver = {
};
module_platform_driver(arm_smmu_driver);
-IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1");
-IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2");
-IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400");
-IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401");
-IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500");
-IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2");
-
MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
MODULE_AUTHOR("Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 85879cfec52f..b128cb4372d3 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1390,5 +1390,3 @@ static int __init exynos_iommu_init(void)
return ret;
}
core_initcall(exynos_iommu_init);
-
-IOMMU_OF_DECLARE(exynos_iommu_of, "samsung,exynos-sysmmu");
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 40ae6e87cb88..f026aa16d5f1 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1108,9 +1108,6 @@ static void __exit ipmmu_exit(void)
subsys_initcall(ipmmu_init);
module_exit(ipmmu_exit);
-IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa");
-IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795");
-
MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 0d3350463a3f..27377742600d 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -877,7 +877,5 @@ static void __exit msm_iommu_driver_exit(void)
subsys_initcall(msm_iommu_driver_init);
module_exit(msm_iommu_driver_exit);
-IOMMU_OF_DECLARE(msm_iommu_of, "qcom,apq8064-iommu");
-
MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Stepan Moskovchenko <stepanm-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>");
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 2aac8387717c..1904ccf9fc4e 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -27,9 +27,6 @@
#define NO_IOMMU 1
-static const struct of_device_id __iommu_of_table_sentinel
- __used __section(__iommu_of_table_end);
-
/**
* of_get_dma_window - Parse *dma-window property and returns 0 if found.
*
@@ -98,19 +95,6 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
}
EXPORT_SYMBOL_GPL(of_get_dma_window);
-static bool of_iommu_driver_present(struct device_node *np)
-{
- /*
- * If the IOMMU still isn't ready by the time we reach init, assume
- * it never will be. We don't want to defer indefinitely, nor attempt
- * to dereference __iommu_of_table after it's been freed.
- */
- if (system_state >= SYSTEM_RUNNING)
- return false;
-
- return of_match_node(&__iommu_of_table, np);
-}
-
static int of_iommu_xlate(struct device *dev,
struct of_phandle_args *iommu_spec)
{
@@ -120,8 +104,7 @@ static int of_iommu_xlate(struct device *dev,
ops = iommu_ops_from_fwnode(fwnode);
if ((ops && !ops->of_xlate) ||
- !of_device_is_available(iommu_spec->np) ||
- (!ops && !of_iommu_driver_present(iommu_spec->np)))
+ !of_device_is_available(iommu_spec->np))
return NO_IOMMU;
err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
diff --git a/drivers/iommu/qcom_iommu.c b/drivers/iommu/qcom_iommu.c
index 65b9c99707f8..fa0f6c39a144 100644
--- a/drivers/iommu/qcom_iommu.c
+++ b/drivers/iommu/qcom_iommu.c
@@ -947,7 +947,5 @@ static void __exit qcom_iommu_exit(void)
module_init(qcom_iommu_init);
module_exit(qcom_iommu_exit);
-IOMMU_OF_DECLARE(qcom_iommu_dev, "qcom,msm-iommu-v1");
-
MODULE_DESCRIPTION("IOMMU API for QCOM IOMMU v1 implementations");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 0468acfa131f..90d37f29c24c 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1284,8 +1284,6 @@ static int __init rk_iommu_init(void)
}
subsys_initcall(rk_iommu_init);
-IOMMU_OF_DECLARE(rk_iommu_of, "rockchip,iommu");
-
MODULE_DESCRIPTION("IOMMU API for Rockchip");
MODULE_AUTHOR("Simon Xue <xxm-TNX95d0MmH7DzftRWevZcw@public.gmane.org> and Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>");
MODULE_ALIAS("platform:rockchip-iommu");
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
index 4fa654e4b5a9..f3d40dd7bb66 100644
--- a/include/linux/of_iommu.h
+++ b/include/linux/of_iommu.h
@@ -32,8 +32,4 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
#endif /* CONFIG_OF_IOMMU */
-extern struct of_device_id __iommu_of_table;
-
-#define IOMMU_OF_DECLARE(name, compat) OF_DECLARE_1(iommu, name, compat, NULL)
-
#endif /* __OF_IOMMU_H */
--
2.17.0
^ permalink raw reply related
* [PATCH v2 8/8] PM / Domains: Stop deferring probe at the end of initcall
From: Rob Herring @ 2018-05-24 17:50 UTC (permalink / raw)
To: Greg Kroah-Hartman, Linus Walleij, Alexander Graf,
Bjorn Andersson, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
Joerg Roedel, Robin Murphy, Mark Brown, Frank Rowand
Cc: linux-kernel, devicetree, boot-architecture, linux-arm-kernel,
Pavel Machek, Len Brown, linux-pm
In-Reply-To: <20180524175024.19874-1-robh@kernel.org>
All PM domain drivers must be built-in (at least those using DT), so
there is no point deferring probe after initcalls are done. Continuing
to defer probe may prevent booting successfully even if managing PM
domains is not required. This can happen if the user failed to enable
the driver or if power-domains are added to a platform's DT, but there
is not yet a driver (e.g. a new DTB with an old kernel).
Call the driver core function driver_deferred_probe_check_init_done()
instead of just returning -EPROBE_DEFER to stop deferring probe when
initcalls are done.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/base/power/domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 1ea0e2502e8e..6398cf786e6a 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2218,7 +2218,7 @@ int genpd_dev_pm_attach(struct device *dev)
mutex_unlock(&gpd_list_lock);
dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
__func__, PTR_ERR(pd));
- return -EPROBE_DEFER;
+ return driver_deferred_probe_check_init_done(dev, true);
}
dev_dbg(dev, "adding to PM domain %s\n", pd->name);
--
2.17.0
^ permalink raw reply related
* RE: [PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver
From: Vaittinen, Matti @ 2018-05-24 17:51 UTC (permalink / raw)
To: Mark Brown
Cc: mturquette@baylibre.com, sboyd@kernel.org, robh+dt@kernel.org,
mark.rutland@arm.com, lee.jones@linaro.org, lgirdwood@gmail.com,
mazziesaccount@gmail.com, linux-clk@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Mutanen, Mikko, Haikola, Heikki
In-Reply-To: <20180524141427.GU4828@sirena.org.uk>
From: Mark Brown [broonie@kernel.org]
> Sent: Thursday, May 24, 2018 5:14 PM
>
> On Thu, May 24, 2018 at 09:00:36AM +0300, Matti Vaittinen wrote:
>
> > @@ -0,0 +1,683 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (C) 2018 ROHM Semiconductors */
> > +/*
> > + * bd71837-regulator.c ROHM BD71837MWV regulator driver
> > + */
> > +#include <linux/kernel.h>
>
> Make the entire comment block a C++ comment so it looks more intentional
I will change this to C++ - but the verison of checkpatch.pl I used did complain
if I used C++ style comments in C-files or if I used C-style comments in header.
I guess the sscript should be fixed unless it is already done.
> and add a blank line before the headers for legibility.
Will do.
> > +static int bd71837_regulator_set_regmap(struct regulator_dev *rdev, int set)
> > +{
> > + int ret = -EINVAL;
> > + struct bd71837_pmic *pmic = rdev->reg_data;
> > +
> > + if (pmic) {
> > + mutex_lock(&pmic->mtx);
> > + if (!set)
> > + ret = regulator_disable_regmap(rdev);
> > + else
> > + ret = regulator_enable_regmap(rdev);
> > + mutex_unlock(&pmic->mtx);
> > + }
>
> This looks very weird - why might we not have a parent PMIC,
I'll remove the check. I've just adopted habit of adding NULL checks
for pointers "jsut in case".
> what is the lock doing and what is this wrapper function intended to do?
This was the other spot which I was unsure how to handle. Datasheet for
the chip says that if voltage is to be changed, the regulator must be
disabled. Thus my voltage changing function checks if regulator is enabled
and disables it for duration of voltage change (if it was enabled). This lock
is used to protect the voltage change so that no one enables the regulator
during voltage change. I don't know what would have been correct way of
doing this, or if disabling regulator for voltage change is Ok - but this was
the only way I could think of. I am again grateful for any tips.
> Similar
> issues apply to the voltage functions, if there's any need for this it
> needs to be better documented but it really doesn't look like a good
> idea.
If the solution with lock and wrapper (to prevent race during state check
and voltage change) is Ok, then I will add comment which explains this,
> > + err =
> > + regmap_update_bits(pmic->mfd->regmap, BD71837_REG_REGLOCK,
> > + (REGLOCK_PWRSEQ | REGLOCK_VREG), 0);
> > + if (err) {
> > + dev_err(&pmic->pdev->dev, "Failed to unlock PMIC (%d)\n", err);
> > + goto err;
> > + } else
> > + dev_dbg(&pmic->pdev->dev, "%s: Unlocked lock register 0x%x\n",
> > + __func__, BD71837_REG_REGLOCK);
>
> There's loads of coding style problems with this code, please refer to
> the coding style - indentation is weird and if there's { } on one side
> of an else it should be on both.
Will fix it.
> > + rdev = regulator_register(desc, &config);
> > + if (IS_ERR(rdev)) {
>
> devm_regulator_regster()
Makes sense. Thanks
Best Regards
Matti Vaittinen
^ permalink raw reply
* Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi
From: Alexandre Belloni @ 2018-05-24 17:56 UTC (permalink / raw)
To: Radu Pirea
Cc: Mark Brown, Andy Shevchenko, devicetree, open list:SERIAL DRIVERS,
Linux Kernel Mailing List, linux-arm Mailing List, linux-spi,
Mark Rutland, Rob Herring, Lee Jones, Greg Kroah-Hartman,
Jiri Slaby, Richard Genoud, Nicolas Ferre
In-Reply-To: <0e6e71e2-f8ac-7889-0d81-8d8a4c15223d@microchip.com>
Hi,
On 24/05/2018 19:04:11+0300, Radu Pirea wrote:
>
>
> On 05/17/2018 07:54 AM, Mark Brown wrote:
> > On Tue, May 15, 2018 at 12:22:24PM +0300, Radu Pirea wrote:
> > > On Mon, 2018-05-14 at 20:38 +0300, Andy Shevchenko wrote:
> >
> > > > So, what is not going as expected in "SPI core takes care of CSs"
> > > > case?
> > > > Did you use oscilloscope for that?
> >
> > > Yes, I used and CSs was not asserted. Anyway, I will will try again.
> >
> > If the core chip select handling is not working properly for some reason
> > then the core chip select handling should be fixed rather than just open
> > coding in your driver - probably it's also broken for other users.
> >
>
> Hi Mark,
>
> I found the fix for cs-gpios. If I change spi_add_device function like
> this(see below) everything is ok.
>
> int spi_add_device(struct spi_device *spi)
>
> ...
>
> if (ctlr->cs_gpios){
> spi->cs_gpio = ctlr->cs_gpios[spi->chip_select];
> if(gpio_is_valid(spi->cs_gpio))
> gpio_direction_output(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
>
> }
>
> ...
>
> return status;
> }
>
> In the subsystem gpio direction of pins is never set and gpio_set_value()
> don't set the direction.
> In my opinion gpio_direction_output() set direction should be called in
> spi_add_device. What do you think? Is ok?
Back in 2014, I was suggesting using devm_gpio_request_one() in
of_spi_register_master(). That would take care of setting the direction
of the GPIO:
https://www.spinics.net/lists/arm-kernel/msg351251.html
I never took the time to create the patch and test though.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 4/9] regulator: bd71837: Devicetree bindings for BD71837 regulators
From: Mark Brown @ 2018-05-24 17:57 UTC (permalink / raw)
To: Vaittinen, Matti
Cc: mturquette@baylibre.com, sboyd@kernel.org, robh+dt@kernel.org,
mark.rutland@arm.com, lee.jones@linaro.org, lgirdwood@gmail.com,
mazziesaccount@gmail.com, linux-clk@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Mutanen, Mikko, Haikola, Heikki
In-Reply-To: <042F8805D2046347BB8420BEAE397A4016C06B47@WILL-MAIL002.REu.RohmEu.com>
[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]
On Thu, May 24, 2018 at 05:30:57PM +0000, Vaittinen, Matti wrote:
> > > On Thu, May 24, 2018 at 08:57:52AM +0300, Matti Vaittinen wrote:
> >
> > > +Required properties:
> > > + - compatible: should be "rohm,bd71837-pmic".
> > > + - regulator-name: should be "buck1", ..., "buck8" and "ldo1", ..., "ldo7"
> >
> > The MFD is for a single device, there should be no need for compatibles
> > on subfunctions.
>
> I will check this. I must admit I am not sure what is the de-facto mechanism
> for assigning the correct device-tree nodes to sub devices if compatibles
> are not used? I think I saw device-tree node name being used for regulators
You can look at the regulators node within the parent device, you know
that in Linux the parent device will be the MFD. Having a compatible
string within the device makes no difference here. There's quite a few
in tree examples of this.
> Also, another thing I was wondering is how supply regulators should be
> handled? In this case the LDO5 is supplied by BUCK6 and LDO6 by
> BUCK7.
> From generic regulator bindings
> /Documentation/devicetree/bindings/regulator/regulator.txt
> I found statement:
> > - <name>-supply: phandle to the parent supply/regulator node
None of that stuff uses compatible strings, just handle it as covered in
the bindings.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 8/9] regulator: bd71837: BD71837 PMIC regulator driver
From: Mark Brown @ 2018-05-24 17:59 UTC (permalink / raw)
To: Vaittinen, Matti
Cc: mturquette@baylibre.com, sboyd@kernel.org, robh+dt@kernel.org,
mark.rutland@arm.com, lee.jones@linaro.org, lgirdwood@gmail.com,
mazziesaccount@gmail.com, linux-clk@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Mutanen, Mikko, Haikola, Heikki
In-Reply-To: <042F8805D2046347BB8420BEAE397A4016C06B60@WILL-MAIL002.REu.RohmEu.com>
[-- Attachment #1: Type: text/plain, Size: 756 bytes --]
On Thu, May 24, 2018 at 05:51:27PM +0000, Vaittinen, Matti wrote:
> > what is the lock doing and what is this wrapper function intended to do?
> This was the other spot which I was unsure how to handle. Datasheet for
> the chip says that if voltage is to be changed, the regulator must be
> disabled. Thus my voltage changing function checks if regulator is enabled
Ugh, this chip is not very good is it? Don't bounce the supply to
change the voltage silently, that's clearly a bad idea - the devices
using the supply are going to get very upset when the power gets removed
just because they changed the voltage. Instead implement a custom set
operation that returns an error if the user attempts to change the
voltage while the regualtor is enabled.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v2] arm64: dts: apq8096-db820c: Removed bt-en-1-8v regulator
From: Thierry Escande @ 2018-05-24 18:01 UTC (permalink / raw)
To: Andy Gross, David Brown, Rob Herring, Mark Rutland,
Catalin Marinas, Will Deacon
Cc: Bjorn Andersson, Niklas Cassel, linux-arm-msm, devicetree,
linux-kernel
This patch removes the unused bt-en-1-8v regulator and moves the
bt_en_gios claim to the pm8994_gpios node.
This bt_en_gpio could have been moved to the bluetooth serial node but
instead this node declares an 'enable' gpio addressing the bt_en_gpio.
This is needed by the Qualcomm QCA6174 WLAN/BT combo chip that needs to
have the bt_en_gpio claimed even if only WLAN is used.
Signed-off-by: Thierry Escande <thierry.escande@linaro.org>
---
Change in v2:
- Rebased on top of [1] posted a few days ago:
[1] https://lkml.org/lkml/2018/5/22/949
"arm64: dts: fix regulator property name for wlan pcie endpoint"
arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi | 2 +-
arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 14 --------------
2 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi
index 6167af955659..a6ad3d7fe655 100644
--- a/arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi
@@ -4,7 +4,7 @@
&pm8994_gpios {
pinctrl-names = "default";
- pinctrl-0 = <&ls_exp_gpio_f>;
+ pinctrl-0 = <&ls_exp_gpio_f &bt_en_gpios>;
ls_exp_gpio_f: pm8994_gpio5 {
pinconf {
diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
index 7ca6e78def55..2c026b8af792 100644
--- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
@@ -171,19 +171,6 @@
pinctrl-0 = <&usb2_vbus_det_gpio>;
};
- bt_en: bt-en-1-8v {
- pinctrl-names = "default";
- pinctrl-0 = <&bt_en_gpios>;
- compatible = "regulator-fixed";
- regulator-name = "bt-en-regulator";
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
-
- /* WLAN card specific delay */
- startup-delay-us = <70000>;
- enable-active-high;
- };
-
wlan_en: wlan-en-1-8v {
pinctrl-names = "default";
pinctrl-0 = <&wlan_en_gpios>;
@@ -204,7 +191,6 @@
status = "okay";
perst-gpio = <&msmgpio 35 GPIO_ACTIVE_LOW>;
vddpe-3v3-supply = <&wlan_en>;
- vddpe1-supply = <&bt_en>;
};
pcie@608000 {
--
2.14.1
^ permalink raw reply related
* Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi
From: Mark Brown @ 2018-05-24 18:15 UTC (permalink / raw)
To: Radu Pirea
Cc: Andy Shevchenko, devicetree, open list:SERIAL DRIVERS,
Linux Kernel Mailing List, linux-arm Mailing List, linux-spi,
Mark Rutland, Rob Herring, Lee Jones, Greg Kroah-Hartman,
Jiri Slaby, Richard Genoud, alexandre.belloni, Nicolas Ferre
In-Reply-To: <0e6e71e2-f8ac-7889-0d81-8d8a4c15223d@microchip.com>
[-- Attachment #1: Type: text/plain, Size: 526 bytes --]
On Thu, May 24, 2018 at 07:04:11PM +0300, Radu Pirea wrote:
> if (ctlr->cs_gpios){
> spi->cs_gpio = ctlr->cs_gpios[spi->chip_select];
> if(gpio_is_valid(spi->cs_gpio))
> gpio_direction_output(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
>
> }
You're expected to request the GPIOs in your driver using one of the
modern request functions like gpio_request_one() (or ideally the GPIO
descriptor APIs now) which combine the direction setting and request
into a single operation.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ 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