* Re: [PATCH v3 03/16] mfd: mfd-core: match device tree node against reg property
From: Lee Jones @ 2020-05-26 7:24 UTC (permalink / raw)
To: Michael Walle
Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Jean Delvare, Guenter Roeck, Thierry Reding,
Uwe Kleine-König, Wim Van Sebroeck, Shawn Guo, Li Yang,
Thomas Gleixner, Jason Cooper, Marc Zyngier, Mark Brown,
Greg Kroah-Hartman, linux-gpio, devicetree, linux-kernel,
linux-hwmon, linux-pwm, linux-watchdog, linux-arm-kernel
In-Reply-To: <159e68b4ce53630ef906b2fcbca925bd@walle.cc>
On Mon, 25 May 2020, Michael Walle wrote:
> Am 2020-05-15 12:28, schrieb Lee Jones:
> > On Thu, 30 Apr 2020, Michael Walle wrote:
> >
> > > Hi Lee,
> > >
> > > Am 2020-04-23 19:45, schrieb Michael Walle:
> > > > There might be multiple children with the device tree compatible, for
> > > > example if a MFD has multiple instances of the same function. In this
> > > > case only the first is matched and the other children get a wrong
> > > > of_node reference.
> > > > Add a new option to match also against the unit address of the child
> > > > node. Additonally, a new helper OF_MFD_CELL_REG is added.
> > >
> > >
> > > Do you think this is feasible? I guess this is the biggest uncertainty
> > > for me at the moment in this patch series.
> >
> > I think it sounds fine in principle. So long as it doesn't change the
> > existing behaviour when of_reg isn't set.
> >
> > > > Signed-off-by: Michael Walle <michael@walle.cc>
> > > > ---
> > > > drivers/mfd/mfd-core.c | 29 ++++++++++++++++++++---------
> > > > include/linux/mfd/core.h | 26 ++++++++++++++++++++------
> > > > 2 files changed, 40 insertions(+), 15 deletions(-)
[...]
> > > > diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
> > > > index d01d1299e49d..c2c0ad6b14f3 100644
> > > > --- a/include/linux/mfd/core.h
> > > > +++ b/include/linux/mfd/core.h
> > > > @@ -13,8 +13,11 @@
> > > > #include <linux/platform_device.h>
> > > >
> > > > #define MFD_RES_SIZE(arr) (sizeof(arr) / sizeof(struct resource))
> > > > +#define MFD_OF_REG_VALID BIT(31)
> >
> > What about 64bit platforms?
>
> The idea was to have this as a logical number. I.e. for now you may only
> have one subdevice per unique compatible string. In fact, if you have a
> look at the ab8500.c, there are multiple "stericsson,ab8500-pwm"
> subdevices. But there is only one DT node for all three of it. I guess
> this works as long as you don't use phandles to reference the pwm node
> in the device tree. Or you don't want to use device tree properties
> per subdevice (for example the "timeout-sec" of a watchdog device).
>
> So to circumvent this, I thought of having the unit-address (and thus
> the "reg" property) to differentiate between multiple subdevices. Now
> there is one special case for me: this board management controller
> might be upgradable and it might change internally. Thus I came up
> with that logical numbering of subdevices. Rob doesn't seem to be a
> fan of that, though. Therefore, having bit 31 as a valid indicator
> leaves you with 2^31 logical devices, which should be enough ;)
>
> Rob proposed to have the internal offset as the unit-address. But
> in that case I can also use devm_of_platform_populate() and don't
> need the OF_MFD_CELL_REG; I'd just parse the reg offset in each
> individual subdevice driver. But like I said, I wanted to keep the
> internal offsets out of the device tree.
Oh, I see what you're doing.
So you're adding an arbitrary ID to the device's reg property in DT?
How is this not a hack?
Why don't you use the full address for identification?
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH 1/2] hwmon: pwm-fan: Add profile support and add remove module support
From: Uwe Kleine-König @ 2020-05-26 7:04 UTC (permalink / raw)
To: Sandipan Patra
Cc: treding, jonathanh, kamil, jdelvare, linux, robh+dt, bbasu,
bbiswas, linux-pwm, linux-hwmon, devicetree, linux-tegra,
linux-kernel
In-Reply-To: <1590469565-14953-1-git-send-email-spatra@nvidia.com>
On Tue, May 26, 2020 at 10:36:04AM +0530, Sandipan Patra wrote:
> This change has 2 parts:
> 1. Add support for profiles mode settings.
> This allows different fan settings for trip point temp/hyst/pwm.
> T194 has multiple fan-profiles support.
>
> 2. Add pwm-fan remove support. This is essential since the config is
> tristate capable.
These two are orthogonal, aren't they? So they belong in two patches.
You have to expand the binding documentation.
> Signed-off-by: Sandipan Patra <spatra@nvidia.com>
> ---
> drivers/hwmon/pwm-fan.c | 112 ++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 100 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 30b7b3e..26db589 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -3,8 +3,10 @@
> * pwm-fan.c - Hwmon driver for fans connected to PWM lines.
> *
> * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + * Copyright (c) 2020, NVIDIA Corporation.
> *
> * Author: Kamil Debski <k.debski@samsung.com>
> + * Author: Sandipan Patra <spatra@nvidia.com>
> */
>
> #include <linux/hwmon.h>
> @@ -21,6 +23,8 @@
> #include <linux/timer.h>
>
> #define MAX_PWM 255
> +/* Based on OF max device tree node name length */
> +#define MAX_PROFILE_NAME_LENGTH 31
>
> struct pwm_fan_ctx {
> struct mutex lock;
> @@ -38,6 +42,12 @@ struct pwm_fan_ctx {
> unsigned int pwm_fan_state;
> unsigned int pwm_fan_max_state;
> unsigned int *pwm_fan_cooling_levels;
> +
> + unsigned int pwm_fan_profiles;
> + const char **fan_profile_names;
> + unsigned int **fan_profile_cooling_levels;
> + unsigned int fan_current_profile;
> +
> struct thermal_cooling_device *cdev;
> };
>
> @@ -227,28 +237,86 @@ static int pwm_fan_of_get_cooling_data(struct device *dev,
> struct pwm_fan_ctx *ctx)
> {
> struct device_node *np = dev->of_node;
> + struct device_node *base_profile = NULL;
> + struct device_node *profile_np = NULL;
> + const char *default_profile = NULL;
> int num, i, ret;
>
> - if (!of_find_property(np, "cooling-levels", NULL))
> - return 0;
> + num = of_property_count_u32_elems(np, "cooling-levels");
> + if (num <= 0) {
> + base_profile = of_get_child_by_name(np, "profiles");
> + if (!base_profile) {
> + dev_err(dev, "Wrong Data\n");
> + return -EINVAL;
> + }
> + }
> +
> + if (base_profile) {
> + ctx->pwm_fan_profiles =
> + of_get_available_child_count(base_profile);
> +
> + if (ctx->pwm_fan_profiles <= 0) {
> + dev_err(dev, "Profiles used but not defined\n");
> + return -EINVAL;
> + }
>
> - ret = of_property_count_u32_elems(np, "cooling-levels");
> - if (ret <= 0) {
> - dev_err(dev, "Wrong data!\n");
> - return ret ? : -EINVAL;
> + ctx->fan_profile_names = devm_kzalloc(dev,
> + sizeof(const char *) * ctx->pwm_fan_profiles,
> + GFP_KERNEL);
> + ctx->fan_profile_cooling_levels = devm_kzalloc(dev,
> + sizeof(int *) * ctx->pwm_fan_profiles,
> + GFP_KERNEL);
> +
> + if (!ctx->fan_profile_names
> + || !ctx->fan_profile_cooling_levels)
> + return -ENOMEM;
> +
> + ctx->fan_current_profile = 0;
> + i = 0;
> + for_each_available_child_of_node(base_profile, profile_np) {
> + num = of_property_count_u32_elems(profile_np,
> + "cooling-levels");
> + if (num <= 0) {
> + dev_err(dev, "No data in cooling-levels inside profile node!\n");
> + return -EINVAL;
> + }
> +
> + of_property_read_string(profile_np, "name",
> + &ctx->fan_profile_names[i]);
> + if (default_profile &&
> + !strncmp(default_profile,
> + ctx->fan_profile_names[i],
> + MAX_PROFILE_NAME_LENGTH))
> + ctx->fan_current_profile = i;
> +
> + ctx->fan_profile_cooling_levels[i] =
> + devm_kzalloc(dev, sizeof(int) * num,
> + GFP_KERNEL);
> + if (!ctx->fan_profile_cooling_levels[i])
> + return -ENOMEM;
> +
> + of_property_read_u32_array(profile_np, "cooling-levels",
> + ctx->fan_profile_cooling_levels[i], num);
> + i++;
> + }
> }
>
> - num = ret;
> ctx->pwm_fan_cooling_levels = devm_kcalloc(dev, num, sizeof(u32),
> GFP_KERNEL);
> if (!ctx->pwm_fan_cooling_levels)
> return -ENOMEM;
>
> - ret = of_property_read_u32_array(np, "cooling-levels",
> - ctx->pwm_fan_cooling_levels, num);
> - if (ret) {
> - dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
> - return ret;
> + if (base_profile) {
> + memcpy(ctx->pwm_fan_cooling_levels,
> + ctx->fan_profile_cooling_levels[ctx->fan_current_profile],
> + num);
> + } else {
> + ret = of_property_read_u32_array(np, "cooling-levels",
> + ctx->pwm_fan_cooling_levels, num);
> + if (ret) {
> + dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
> + return -EINVAL;
> + }
> }
>
> for (i = 0; i < num; i++) {
> @@ -390,6 +458,25 @@ static int pwm_fan_probe(struct platform_device *pdev)
> return 0;
> }
>
> +static int pwm_fan_remove(struct platform_device *pdev)
> +{
> + struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev);
> + struct pwm_args args;
> +
> + if (!ctx)
> + return -EINVAL;
> +
> + if (IS_ENABLED(CONFIG_THERMAL))
> + thermal_cooling_device_unregister(ctx->cdev);
> +
> + pwm_get_args(ctx->pwm, &args);
> + pwm_config(ctx->pwm, 0, args.period);
> + pwm_disable(ctx->pwm);
What is what you really here? Is it only that the PWM stops oscillating,
or is it crucial that the output goes to its inactive level?
(The intended semantic of pwm_disable includes that the output goes low,
but not all implementations enforce this.)
Also please don't introduce new users of pwm_config() and pwm_disable()
use pwm_apply() instead.
I wonder if this unregistration is "safe". When the driver is in use I'd
expect that the hwmon device doesn't go away and so the devm
unregistration callback that belongs to
devm_hwmon_device_register_with_groups() blocks. But at this time the
PWM is already stopped and so the device stops functioning earlier.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH v2 6/6] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml
From: Geert Uytterhoeven @ 2020-05-26 7:03 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Ricardo Cañuelo, Collabora Kernel ML,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, Geert Uytterhoeven, Rob Herring, Wei Xu
In-Reply-To: <20200526014444.GB6179@pendragon.ideasonboard.com>
Hi Laurent,
On Tue, May 26, 2020 at 3:44 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Mon, May 25, 2020 at 09:43:35AM +0200, Ricardo Cañuelo wrote:
> > On jue 14-05-2020 18:22:39, Laurent Pinchart wrote:
> > > > If we want to be more strict and require the definition of all the
> > > > supplies, there will be many more DTs changes in the series, and I'm not
> > > > sure I'll be able to do that in a reasonable amount of time. I'm looking
> > > > at them and it's not always clear which regulators to use or if they are
> > > > even defined.
> > >
> > > We can decouple the two though (I think). The bindings should reflect
> > > what we consider right, and the dts files could be fixed on top.
> >
> > Do you have a suggestion on how to do this? If we decouple the two
> > tasks most of the work would be searching for DTs to fix and finding a
> > way to fix each one of them, and unless I do this _before_ the binding
> > conversion I'll get a lot of dtbs_check errors.
>
> Rob should answer this question as it will be his decision, but I've
> personally never considered non-compliant DT sources to be an obstacle
> to bindings conversion to YAML. The DT sources should be fixed, but I
> don't see it as a prerequisite (although it's a good practice).
I do my best to avoid introducing regressions when the binding conversions
go upstream.
FTR, hence patches 1-3 are already in v5.7-rc7.
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: [PATCHv3 0/2] Add support for replicators which loses context on clock removal
From: Sai Prakash Ranjan @ 2020-05-26 7:01 UTC (permalink / raw)
To: Mathieu Poirier, devicetree, Rob Herring
Cc: Suzuki K Poulose, Mike Leach, linux-arm-kernel, linux-kernel,
linux-arm-msm, Stephen Boyd, devicetree-owner
In-Reply-To: <20200525161553.GA31527@xps15>
Hi Mathieu,
On 2020-05-25 21:45, Mathieu Poirier wrote:
> Hi Sai,
>
> On Sat, May 23, 2020 at 12:06:50AM +0530, Sai Prakash Ranjan wrote:
>> This series is mainly to add support for replicators
>> which lose context on removing AMBA clock like on SC7180
>> SoC where replicator in AOSS domain loses context.
>>
>
> I am good with this set but need a reviewed-by on the DT binding before
> I can
> add it to my tree. The same goes for your other set[1].
>
Sure, let's wait for an ack from Rob and thanks for the reviews.
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
* [PATCH v2 3/3] dt-bindings: mmc: convert arasan sdhci bindings to yaml
From: Wan Ahmad Zainie @ 2020-05-26 6:27 UTC (permalink / raw)
To: ulf.hansson, robh+dt, adrian.hunter, michal.simek
Cc: linux-mmc, devicetree, wan.ahmad.zainie.wan.mohamad
In-Reply-To: <20200526062758.17642-1-wan.ahmad.zainie.wan.mohamad@intel.com>
Convert arasan,sdhci.txt file to yaml. The new file arasan,sdhci.yaml
will inherit properties from mmc-controller.yaml. 'sdhci' is no longer
a valid name for node and should be changed to 'mmc'.
Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
---
.../devicetree/bindings/mmc/arasan,sdhci.txt | 192 ------------
.../devicetree/bindings/mmc/arasan,sdhci.yaml | 293 ++++++++++++++++++
2 files changed, 293 insertions(+), 192 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
create mode 100644 Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
deleted file mode 100644
index f29bf7dd2ece..000000000000
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ /dev/null
@@ -1,192 +0,0 @@
-Device Tree Bindings for the Arasan SDHCI Controller
-
- The bindings follow the mmc[1], clock[2], interrupt[3] and phy[4] bindings.
- Only deviations are documented here.
-
- [1] Documentation/devicetree/bindings/mmc/mmc.txt
- [2] Documentation/devicetree/bindings/clock/clock-bindings.txt
- [3] Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
- [4] Documentation/devicetree/bindings/phy/phy-bindings.txt
-
-Required Properties:
- - compatible: Compatibility string. One of:
- - "arasan,sdhci-8.9a": generic Arasan SDHCI 8.9a PHY
- - "arasan,sdhci-4.9a": generic Arasan SDHCI 4.9a PHY
- - "arasan,sdhci-5.1": generic Arasan SDHCI 5.1 PHY
- - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY
- For this device it is strongly suggested to include arasan,soc-ctl-syscon.
- - "xlnx,zynqmp-8.9a": ZynqMP SDHCI 8.9a PHY
- For this device it is strongly suggested to include clock-output-names and
- #clock-cells.
- - "xlnx,versal-8.9a": Versal SDHCI 8.9a PHY
- For this device it is strongly suggested to include clock-output-names and
- #clock-cells.
- - "ti,am654-sdhci-5.1", "arasan,sdhci-5.1": TI AM654 MMC PHY
- Note: This binding has been deprecated and moved to [5].
- - "intel,lgm-sdhci-5.1-emmc", "arasan,sdhci-5.1": Intel LGM eMMC PHY
- For this device it is strongly suggested to include arasan,soc-ctl-syscon.
- - "intel,lgm-sdhci-5.1-sdxc", "arasan,sdhci-5.1": Intel LGM SDXC PHY
- For this device it is strongly suggested to include arasan,soc-ctl-syscon.
- - "intel,keembay-sdhci-5.1-emmc", "arasan,sdhci-5.1": Intel Keem Bay eMMC
- For this device it is strongly suggested to include arasan,soc-ctl-syscon.
- - "intel,keembay-sdhci-5.1-sd": Intel Keem Bay SD controller
- For this device it is strongly suggested to include arasan,soc-ctl-syscon.
- - "intel,keembay-sdhci-5.1-sdio": Intel Keem Bay SDIO controller
- For this device it is strongly suggested to include arasan,soc-ctl-syscon.
-
- [5] Documentation/devicetree/bindings/mmc/sdhci-am654.txt
-
- - reg: From mmc bindings: Register location and length.
- - clocks: From clock bindings: Handles to clock inputs.
- - clock-names: From clock bindings: Tuple including "clk_xin" and "clk_ahb"
- - interrupts: Interrupt specifier
-
-Required Properties for "arasan,sdhci-5.1":
- - phys: From PHY bindings: Phandle for the Generic PHY for arasan.
- - phy-names: MUST be "phy_arasan".
-
-Optional Properties:
- - arasan,soc-ctl-syscon: A phandle to a syscon device (see ../mfd/syscon.txt)
- used to access core corecfg registers. Offsets of registers in this
- syscon are determined based on the main compatible string for the device.
- - clock-output-names: If specified, this will be the name of the card clock
- which will be exposed by this device. Required if #clock-cells is
- specified.
- - #clock-cells: If specified this should be the value <0> or <1>. With this
- property in place we will export one or two clocks representing the Card
- Clock. These clocks are expected to be consumed by our PHY.
- - xlnx,fails-without-test-cd: when present, the controller doesn't work when
- the CD line is not connected properly, and the line is not connected
- properly. Test mode can be used to force the controller to function.
- - xlnx,int-clock-stable-broken: when present, the controller always reports
- that the internal clock is stable even when it is not.
-
- - xlnx,mio-bank: When specified, this will indicate the MIO bank number in
- which the command and data lines are configured. If not specified, driver
- will assume this as 0.
-
-Example:
- sdhci@e0100000 {
- compatible = "arasan,sdhci-8.9a";
- reg = <0xe0100000 0x1000>;
- clock-names = "clk_xin", "clk_ahb";
- clocks = <&clkc 21>, <&clkc 32>;
- interrupt-parent = <&gic>;
- interrupts = <0 24 4>;
- } ;
-
- sdhci@e2800000 {
- compatible = "arasan,sdhci-5.1";
- reg = <0xe2800000 0x1000>;
- clock-names = "clk_xin", "clk_ahb";
- clocks = <&cru 8>, <&cru 18>;
- interrupt-parent = <&gic>;
- interrupts = <0 24 4>;
- phys = <&emmc_phy>;
- phy-names = "phy_arasan";
- } ;
-
- sdhci: sdhci@fe330000 {
- compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
- reg = <0x0 0xfe330000 0x0 0x10000>;
- interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
- clock-names = "clk_xin", "clk_ahb";
- arasan,soc-ctl-syscon = <&grf>;
- assigned-clocks = <&cru SCLK_EMMC>;
- assigned-clock-rates = <200000000>;
- clock-output-names = "emmc_cardclock";
- phys = <&emmc_phy>;
- phy-names = "phy_arasan";
- #clock-cells = <0>;
- };
-
- sdhci: mmc@ff160000 {
- compatible = "xlnx,zynqmp-8.9a", "arasan,sdhci-8.9a";
- interrupt-parent = <&gic>;
- interrupts = <0 48 4>;
- reg = <0x0 0xff160000 0x0 0x1000>;
- clocks = <&clk200>, <&clk200>;
- clock-names = "clk_xin", "clk_ahb";
- clock-output-names = "clk_out_sd0", "clk_in_sd0";
- #clock-cells = <1>;
- clk-phase-sd-hs = <63>, <72>;
- };
-
- sdhci: mmc@f1040000 {
- compatible = "xlnx,versal-8.9a", "arasan,sdhci-8.9a";
- interrupt-parent = <&gic>;
- interrupts = <0 126 4>;
- reg = <0x0 0xf1040000 0x0 0x10000>;
- clocks = <&clk200>, <&clk200>;
- clock-names = "clk_xin", "clk_ahb";
- clock-output-names = "clk_out_sd0", "clk_in_sd0";
- #clock-cells = <1>;
- clk-phase-sd-hs = <132>, <60>;
- };
-
- emmc: sdhci@ec700000 {
- compatible = "intel,lgm-sdhci-5.1-emmc", "arasan,sdhci-5.1";
- reg = <0xec700000 0x300>;
- interrupt-parent = <&ioapic1>;
- interrupts = <44 1>;
- clocks = <&cgu0 LGM_CLK_EMMC5>, <&cgu0 LGM_CLK_NGI>,
- <&cgu0 LGM_GCLK_EMMC>;
- clock-names = "clk_xin", "clk_ahb", "gate";
- clock-output-names = "emmc_cardclock";
- #clock-cells = <0>;
- phys = <&emmc_phy>;
- phy-names = "phy_arasan";
- arasan,soc-ctl-syscon = <&sysconf>;
- };
-
- sdxc: sdhci@ec600000 {
- compatible = "arasan,sdhci-5.1", "intel,lgm-sdhci-5.1-sdxc";
- reg = <0xec600000 0x300>;
- interrupt-parent = <&ioapic1>;
- interrupts = <43 1>;
- clocks = <&cgu0 LGM_CLK_SDIO>, <&cgu0 LGM_CLK_NGI>,
- <&cgu0 LGM_GCLK_SDXC>;
- clock-names = "clk_xin", "clk_ahb", "gate";
- clock-output-names = "sdxc_cardclock";
- #clock-cells = <0>;
- phys = <&sdxc_phy>;
- phy-names = "phy_arasan";
- arasan,soc-ctl-syscon = <&sysconf>;
- };
-
- mmc: mmc@33000000 {
- compatible = "intel,keembay-sdhci-5.1-emmc", "arasan,sdhci-5.1";
- interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
- reg = <0x0 0x33000000 0x0 0x300>;
- clock-names = "clk_xin", "clk_ahb";
- clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>,
- <&scmi_clk KEEM_BAY_PSS_EMMC>;
- phys = <&emmc_phy>;
- phy-names = "phy_arasan";
- assigned-clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>;
- assigned-clock-rates = <200000000>;
- clock-output-names = "emmc_cardclock";
- #clock-cells = <0>;
- arasan,soc-ctl-syscon = <&mmc_phy_syscon>;
- };
-
- sd0: mmc@31000000 {
- compatible = "intel,keembay-sdhci-5.1-sd";
- interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
- reg = <0x0 0x31000000 0x0 0x300>;
- clock-names = "clk_xin", "clk_ahb";
- clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD0>,
- <&scmi_clk KEEM_BAY_PSS_SD0>;
- arasan,soc-ctl-syscon = <&sd0_phy_syscon>;
- };
-
- sd1: mmc@32000000 {
- compatible = "intel,keembay-sdhci-5.1-sdio";
- interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
- reg = <0x0 0x32000000 0x0 0x300>;
- clock-names = "clk_xin", "clk_ahb";
- clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD1>,
- <&scmi_clk KEEM_BAY_PSS_SD1>;
- arasan,soc-ctl-syscon = <&sd1_phy_syscon>;
- };
diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
new file mode 100644
index 000000000000..927e2f13958b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
@@ -0,0 +1,293 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/mmc/arasan,sdhci.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Device Tree Bindings for the Arasan SDHCI Controller
+
+allOf:
+ - $ref: "mmc-controller.yaml#"
+
+maintainers:
+ - Adrian Hunter <adrian.hunter@intel.com>
+
+properties:
+ compatible:
+ oneOf:
+ - const: arasan,sdhci-8.9a # generic Arasan SDHCI 8.9a PHY
+ - const: arasan,sdhci-4.9a # generic Arasan SDHCI 4.9a PHY
+ - const: arasan,sdhci-5.1 # generic Arasan SDHCI 5.1 PHY
+ - items:
+ - const: rockchip,rk3399-sdhci-5.1 # rk3399 eMMC PHY
+ - const: arasan,sdhci-5.1
+ description: |
+ For this device it is strongly suggested to include
+ arasan,soc-ctl-syscon.
+ - items:
+ - const: xlnx,zynqmp-8.9a # ZynqMP SDHCI 8.9a PHY
+ - const: arasan,sdhci-8.9a
+ description: |
+ For this device it is strongly suggested to include
+ clock-output-names and '#clock-cells'.
+ - items:
+ - const: xlnx,versal-8.9a # Versal SDHCI 8.9a PHY
+ - const: arasan,sdhci-8.9a
+ description: |
+ For this device it is strongly suggested to include
+ clock-output-names and '#clock-cells'.
+ - items:
+ - const: intel,lgm-sdhci-5.1-emmc # Intel LGM eMMC PHY
+ - const: arasan,sdhci-5.1
+ description: |
+ For this device it is strongly suggested to include
+ arasan,soc-ctl-syscon.
+ - items:
+ - const: intel,lgm-sdhci-5.1-sdxc # Intel LGM SDXC PHY
+ - const: arasan,sdhci-5.1
+ description: |
+ For this device it is strongly suggested to include
+ arasan,soc-ctl-syscon.
+ - items:
+ - const: intel,keembay-sdhci-5.1-emmc # Intel Keem Bay eMMC PHY
+ - const: arasan,sdhci-5.1
+ description: |
+ For this device it is strongly suggested to include
+ arasan,soc-ctl-syscon.
+ - const: intel,keembay-sdhci-5.1-sd # Intel Keem Bay SD controller
+ description: |
+ For this device it is strongly suggested to include
+ arasan,soc-ctl-syscon.
+ - const: intel,keembay-sdhci-5.1-sdio # Intel Keem Bay SDIO controller
+ description: |
+ For this device it is strongly suggested to include
+ arasan,soc-ctl-syscon.
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ minItems: 2
+ maxItems: 3
+
+ clock-names:
+ minItems: 2
+ items:
+ - const: clk_xin
+ - const: clk_ahb
+ - const: gate
+
+ interrupts:
+ maxItems: 1
+
+ phys:
+ maxItems: 1
+
+ phy-names:
+ const: phy_arasan
+
+ arasan,soc-ctl-syscon:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: |
+ A phandle to a syscon device (see ../mfd/syscon.txt) used to access
+ core corecfg registers. Offsets of registers in this syscon are
+ determined based on the main compatible string for the device.
+
+ clock-output-names:
+ description: |
+ If specified, this will be the name of the card clock which will
+ be exposed by this device. Required if '#clock-cells' is specified.
+
+ '#clock-cells':
+ enum: [0, 1]
+ description: |
+ With this property in place we will export one or two clocks
+ representing the Card Clock. These clocks are expected to be
+ consumed by our PHY.
+
+ xlnx,fails-without-test-cd:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ When present, the controller doesn't work when the CD line is not
+ connected properly, and the line is not connected properly.
+ Test mode can be used to force the controller to function.
+
+ xlnx,int-clock-stable-broken:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ When present, the controller always reports that the internal clock
+ is stable even when it is not.
+
+ xlnx,mio-bank:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ When specified, this will indicate the MIO bank number in which
+ the command and data lines are configured. If not specified, driver
+ will assume this as 0.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+if:
+ properties:
+ compatible:
+ contains:
+ const: arasan,sdhci-5.1
+then:
+ required:
+ - phys
+ - phy-names
+
+examples:
+ - |
+ mmc@e0100000 {
+ compatible = "arasan,sdhci-8.9a";
+ reg = <0xe0100000 0x1000>;
+ clock-names = "clk_xin", "clk_ahb";
+ clocks = <&clkc 21>, <&clkc 32>;
+ interrupt-parent = <&gic>;
+ interrupts = <0 24 4>;
+ };
+
+ - |
+ mmc@e2800000 {
+ compatible = "arasan,sdhci-5.1";
+ reg = <0xe2800000 0x1000>;
+ clock-names = "clk_xin", "clk_ahb";
+ clocks = <&cru 8>, <&cru 18>;
+ interrupt-parent = <&gic>;
+ interrupts = <0 24 4>;
+ phys = <&emmc_phy>;
+ phy-names = "phy_arasan";
+ };
+
+ - |
+ #include <dt-bindings/clock/rk3399-cru.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ mmc@fe330000 {
+ compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
+ reg = <0x0 0xfe330000 0x0 0x10000>;
+ interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
+ clock-names = "clk_xin", "clk_ahb";
+ arasan,soc-ctl-syscon = <&grf>;
+ assigned-clocks = <&cru SCLK_EMMC>;
+ assigned-clock-rates = <200000000>;
+ clock-output-names = "emmc_cardclock";
+ phys = <&emmc_phy>;
+ phy-names = "phy_arasan";
+ #clock-cells = <0>;
+ };
+
+ - |
+ mmc@ff160000 {
+ compatible = "xlnx,zynqmp-8.9a", "arasan,sdhci-8.9a";
+ interrupt-parent = <&gic>;
+ interrupts = <0 48 4>;
+ reg = <0x0 0xff160000 0x0 0x1000>;
+ clocks = <&clk200>, <&clk200>;
+ clock-names = "clk_xin", "clk_ahb";
+ clock-output-names = "clk_out_sd0", "clk_in_sd0";
+ #clock-cells = <1>;
+ clk-phase-sd-hs = <63 72>;
+ };
+
+ - |
+ mmc@f1040000 {
+ compatible = "xlnx,versal-8.9a", "arasan,sdhci-8.9a";
+ interrupt-parent = <&gic>;
+ interrupts = <0 126 4>;
+ reg = <0x0 0xf1040000 0x0 0x10000>;
+ clocks = <&clk200>, <&clk200>;
+ clock-names = "clk_xin", "clk_ahb";
+ clock-output-names = "clk_out_sd0", "clk_in_sd0";
+ #clock-cells = <1>;
+ clk-phase-sd-hs = <132>, <60>;
+ };
+
+ - |
+ #define LGM_CLK_EMMC5
+ #define LGM_CLK_NGI
+ #define LGM_GCLK_EMMC
+ mmc@ec700000 {
+ compatible = "intel,lgm-sdhci-5.1-emmc", "arasan,sdhci-5.1";
+ reg = <0xec700000 0x300>;
+ interrupt-parent = <&ioapic1>;
+ interrupts = <44 1>;
+ clocks = <&cgu0 LGM_CLK_EMMC5>, <&cgu0 LGM_CLK_NGI>,
+ <&cgu0 LGM_GCLK_EMMC>;
+ clock-names = "clk_xin", "clk_ahb", "gate";
+ clock-output-names = "emmc_cardclock";
+ #clock-cells = <0>;
+ phys = <&emmc_phy>;
+ phy-names = "phy_arasan";
+ arasan,soc-ctl-syscon = <&sysconf>;
+ };
+
+ - |
+ #define LGM_CLK_SDIO
+ #define LGM_GCLK_SDXC
+ mmc@ec600000 {
+ compatible = "intel,lgm-sdhci-5.1-sdxc", "arasan,sdhci-5.1";
+ reg = <0xec600000 0x300>;
+ interrupt-parent = <&ioapic1>;
+ interrupts = <43 1>;
+ clocks = <&cgu0 LGM_CLK_SDIO>, <&cgu0 LGM_CLK_NGI>,
+ <&cgu0 LGM_GCLK_SDXC>;
+ clock-names = "clk_xin", "clk_ahb", "gate";
+ clock-output-names = "sdxc_cardclock";
+ #clock-cells = <0>;
+ phys = <&sdxc_phy>;
+ phy-names = "phy_arasan";
+ arasan,soc-ctl-syscon = <&sysconf>;
+ };
+
+ - |
+ #define KEEM_BAY_PSS_AUX_EMMC
+ #define KEEM_BAY_PSS_EMMC
+ mmc@33000000 {
+ compatible = "intel,keembay-sdhci-5.1-emmc", "arasan,sdhci-5.1";
+ interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x0 0x33000000 0x0 0x300>;
+ clock-names = "clk_xin", "clk_ahb";
+ clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>,
+ <&scmi_clk KEEM_BAY_PSS_EMMC>;
+ phys = <&emmc_phy>;
+ phy-names = "phy_arasan";
+ assigned-clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>;
+ assigned-clock-rates = <200000000>;
+ clock-output-names = "emmc_cardclock";
+ #clock-cells = <0>;
+ arasan,soc-ctl-syscon = <&mmc_phy_syscon>;
+ };
+
+ - |
+ #define KEEM_BAY_PSS_AUX_SD0
+ #define KEEM_BAY_PSS_SD0
+ mmc@31000000 {
+ compatible = "intel,keembay-sdhci-5.1-sd";
+ interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x0 0x31000000 0x0 0x300>;
+ clock-names = "clk_xin", "clk_ahb";
+ clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD0>,
+ <&scmi_clk KEEM_BAY_PSS_SD0>;
+ arasan,soc-ctl-syscon = <&sd0_phy_syscon>;
+ };
+
+ - |
+ #define KEEM_BAY_PSS_AUX_SD1
+ #define KEEM_BAY_PSS_SD1
+ mmc@32000000 {
+ compatible = "intel,keembay-sdhci-5.1-sdio";
+ interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x0 0x32000000 0x0 0x300>;
+ clock-names = "clk_xin", "clk_ahb";
+ clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD1>,
+ <&scmi_clk KEEM_BAY_PSS_SD1>;
+ arasan,soc-ctl-syscon = <&sd1_phy_syscon>;
+ };
--
2.17.1
^ permalink raw reply related
* [PATCH v2 2/3] mmc: sdhci-of-arasan: Add support for Intel Keem Bay
From: Wan Ahmad Zainie @ 2020-05-26 6:27 UTC (permalink / raw)
To: ulf.hansson, robh+dt, adrian.hunter, michal.simek
Cc: linux-mmc, devicetree, wan.ahmad.zainie.wan.mohamad
In-Reply-To: <20200526062758.17642-1-wan.ahmad.zainie.wan.mohamad@intel.com>
Intel Keem Bay SoC eMMC/SD/SDIO controller is based on
Arasan SD 3.0 / eMMC 5.1 host controller IP.
However, it does not support 64-bit access as its AXI interface
has 32-bit address ports.
Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/host/sdhci-of-arasan.c | 123 +++++++++++++++++++++++++++++
1 file changed, 123 insertions(+)
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 2fe2c4dcc280..db9b544465cd 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -75,6 +75,7 @@ struct sdhci_arasan_soc_ctl_field {
*
* @baseclkfreq: Where to find corecfg_baseclkfreq
* @clockmultiplier: Where to find corecfg_clockmultiplier
+ * @support64b: Where to find SUPPORT64B bit
* @hiword_update: If true, use HIWORD_UPDATE to access the syscon
*
* It's up to the licensee of the Arsan IP block to make these available
@@ -84,6 +85,7 @@ struct sdhci_arasan_soc_ctl_field {
struct sdhci_arasan_soc_ctl_map {
struct sdhci_arasan_soc_ctl_field baseclkfreq;
struct sdhci_arasan_soc_ctl_field clockmultiplier;
+ struct sdhci_arasan_soc_ctl_field support64b;
bool hiword_update;
};
@@ -180,6 +182,13 @@ static const struct sdhci_arasan_soc_ctl_map intel_lgm_sdxc_soc_ctl_map = {
.hiword_update = false,
};
+static const struct sdhci_arasan_soc_ctl_map intel_keembay_soc_ctl_map = {
+ .baseclkfreq = { .reg = 0x0, .width = 8, .shift = 14 },
+ .clockmultiplier = { .reg = 0x4, .width = 8, .shift = 14 },
+ .support64b = { .reg = 0x4, .width = 1, .shift = 24 },
+ .hiword_update = false,
+};
+
/**
* sdhci_arasan_syscon_write - Write to a field in soc_ctl registers
*
@@ -1095,6 +1104,50 @@ static struct sdhci_arasan_of_data sdhci_arasan_generic_data = {
.clk_ops = &arasan_clk_ops,
};
+static const struct sdhci_pltfm_data sdhci_keembay_emmc_pdata = {
+ .ops = &sdhci_arasan_cqe_ops,
+ .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
+ SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
+ SDHCI_QUIRK_NO_LED |
+ SDHCI_QUIRK_32BIT_DMA_ADDR |
+ SDHCI_QUIRK_32BIT_DMA_SIZE |
+ SDHCI_QUIRK_32BIT_ADMA_SIZE,
+ .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+ SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN |
+ SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400 |
+ SDHCI_QUIRK2_STOP_WITH_TC |
+ SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
+};
+
+static const struct sdhci_pltfm_data sdhci_keembay_sd_pdata = {
+ .ops = &sdhci_arasan_ops,
+ .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
+ SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
+ SDHCI_QUIRK_NO_LED |
+ SDHCI_QUIRK_32BIT_DMA_ADDR |
+ SDHCI_QUIRK_32BIT_DMA_SIZE |
+ SDHCI_QUIRK_32BIT_ADMA_SIZE,
+ .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+ SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN |
+ SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON |
+ SDHCI_QUIRK2_STOP_WITH_TC |
+ SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
+};
+
+static const struct sdhci_pltfm_data sdhci_keembay_sdio_pdata = {
+ .ops = &sdhci_arasan_ops,
+ .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN |
+ SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
+ SDHCI_QUIRK_NO_LED |
+ SDHCI_QUIRK_32BIT_DMA_ADDR |
+ SDHCI_QUIRK_32BIT_DMA_SIZE |
+ SDHCI_QUIRK_32BIT_ADMA_SIZE,
+ .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+ SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN |
+ SDHCI_QUIRK2_HOST_OFF_CARD_ON |
+ SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
+};
+
static struct sdhci_arasan_of_data sdhci_arasan_rk3399_data = {
.soc_ctl_map = &rk3399_soc_ctl_map,
.pdata = &sdhci_arasan_cqe_pdata,
@@ -1140,6 +1193,21 @@ static struct sdhci_arasan_of_data sdhci_arasan_versal_data = {
.clk_ops = &versal_clk_ops,
};
+static struct sdhci_arasan_of_data intel_keembay_emmc_data = {
+ .soc_ctl_map = &intel_keembay_soc_ctl_map,
+ .pdata = &sdhci_keembay_emmc_pdata,
+};
+
+static struct sdhci_arasan_of_data intel_keembay_sd_data = {
+ .soc_ctl_map = &intel_keembay_soc_ctl_map,
+ .pdata = &sdhci_keembay_sd_pdata,
+};
+
+static struct sdhci_arasan_of_data intel_keembay_sdio_data = {
+ .soc_ctl_map = &intel_keembay_soc_ctl_map,
+ .pdata = &sdhci_keembay_sdio_pdata,
+};
+
static const struct of_device_id sdhci_arasan_of_match[] = {
/* SoC-specific compatible strings w/ soc_ctl_map */
{
@@ -1154,6 +1222,18 @@ static const struct of_device_id sdhci_arasan_of_match[] = {
.compatible = "intel,lgm-sdhci-5.1-sdxc",
.data = &intel_lgm_sdxc_data,
},
+ {
+ .compatible = "intel,keembay-sdhci-5.1-emmc",
+ .data = &intel_keembay_emmc_data,
+ },
+ {
+ .compatible = "intel,keembay-sdhci-5.1-sd",
+ .data = &intel_keembay_sd_data,
+ },
+ {
+ .compatible = "intel,keembay-sdhci-5.1-sdio",
+ .data = &intel_keembay_sdio_data,
+ },
/* Generic compatible below here */
{
.compatible = "arasan,sdhci-8.9a",
@@ -1297,6 +1377,40 @@ static void sdhci_arasan_unregister_sdclk(struct device *dev)
of_clk_del_provider(dev->of_node);
}
+/**
+ * sdhci_arasan_update_support64b - Set SUPPORT_64B (64-bit System Bus Support)
+ *
+ * This should be set based on the System Address Bus.
+ * 0: the Core supports only 32-bit System Address Bus.
+ * 1: the Core supports 64-bit System Address Bus.
+ *
+ * NOTES:
+ * - For Keem Bay, it is required to clear this bit. Its default value is 1'b1.
+ * Keem Bay does not support 64-bit access.
+ *
+ * @host The sdhci_host
+ */
+static void sdhci_arasan_update_support64b(struct sdhci_host *host, u32 value)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
+ const struct sdhci_arasan_soc_ctl_map *soc_ctl_map =
+ sdhci_arasan->soc_ctl_map;
+
+ /* Having a map is optional */
+ if (!soc_ctl_map)
+ return;
+
+ /* If we have a map, we expect to have a syscon */
+ if (!sdhci_arasan->soc_ctl_base) {
+ pr_warn("%s: Have regmap, but no soc-ctl-syscon\n",
+ mmc_hostname(host->mmc));
+ return;
+ }
+
+ sdhci_arasan_syscon_write(host, &soc_ctl_map->support64b, value);
+}
+
/**
* sdhci_arasan_register_sdclk - Register the sdcardclk for a PHY to use
*
@@ -1469,6 +1583,15 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
"rockchip,rk3399-sdhci-5.1"))
sdhci_arasan_update_clockmultiplier(host, 0x0);
+ if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-emmc") ||
+ of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd") ||
+ of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sdio")) {
+ sdhci_arasan_update_clockmultiplier(host, 0x0);
+ sdhci_arasan_update_support64b(host, 0x0);
+
+ host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
+ }
+
sdhci_arasan_update_baseclkfreq(host);
ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, &pdev->dev);
--
2.17.1
^ permalink raw reply related
* [PATCH v2 1/3] dt-bindings: mmc: arasan: Add compatible strings for Intel Keem Bay
From: Wan Ahmad Zainie @ 2020-05-26 6:27 UTC (permalink / raw)
To: ulf.hansson, robh+dt, adrian.hunter, michal.simek
Cc: linux-mmc, devicetree, wan.ahmad.zainie.wan.mohamad
In-Reply-To: <20200526062758.17642-1-wan.ahmad.zainie.wan.mohamad@intel.com>
Add new compatible strings in sdhci-of-arasan.c to support Intel Keem Bay
eMMC/SD/SDIO controller, based on Arasan SDHCI 5.1.
Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
---
.../devicetree/bindings/mmc/arasan,sdhci.txt | 42 +++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index 630fe707f5c4..f29bf7dd2ece 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -27,6 +27,12 @@ Required Properties:
For this device it is strongly suggested to include arasan,soc-ctl-syscon.
- "intel,lgm-sdhci-5.1-sdxc", "arasan,sdhci-5.1": Intel LGM SDXC PHY
For this device it is strongly suggested to include arasan,soc-ctl-syscon.
+ - "intel,keembay-sdhci-5.1-emmc", "arasan,sdhci-5.1": Intel Keem Bay eMMC
+ For this device it is strongly suggested to include arasan,soc-ctl-syscon.
+ - "intel,keembay-sdhci-5.1-sd": Intel Keem Bay SD controller
+ For this device it is strongly suggested to include arasan,soc-ctl-syscon.
+ - "intel,keembay-sdhci-5.1-sdio": Intel Keem Bay SDIO controller
+ For this device it is strongly suggested to include arasan,soc-ctl-syscon.
[5] Documentation/devicetree/bindings/mmc/sdhci-am654.txt
@@ -148,3 +154,39 @@ Example:
phy-names = "phy_arasan";
arasan,soc-ctl-syscon = <&sysconf>;
};
+
+ mmc: mmc@33000000 {
+ compatible = "intel,keembay-sdhci-5.1-emmc", "arasan,sdhci-5.1";
+ interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x0 0x33000000 0x0 0x300>;
+ clock-names = "clk_xin", "clk_ahb";
+ clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>,
+ <&scmi_clk KEEM_BAY_PSS_EMMC>;
+ phys = <&emmc_phy>;
+ phy-names = "phy_arasan";
+ assigned-clocks = <&scmi_clk KEEM_BAY_PSS_AUX_EMMC>;
+ assigned-clock-rates = <200000000>;
+ clock-output-names = "emmc_cardclock";
+ #clock-cells = <0>;
+ arasan,soc-ctl-syscon = <&mmc_phy_syscon>;
+ };
+
+ sd0: mmc@31000000 {
+ compatible = "intel,keembay-sdhci-5.1-sd";
+ interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x0 0x31000000 0x0 0x300>;
+ clock-names = "clk_xin", "clk_ahb";
+ clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD0>,
+ <&scmi_clk KEEM_BAY_PSS_SD0>;
+ arasan,soc-ctl-syscon = <&sd0_phy_syscon>;
+ };
+
+ sd1: mmc@32000000 {
+ compatible = "intel,keembay-sdhci-5.1-sdio";
+ interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0x0 0x32000000 0x0 0x300>;
+ clock-names = "clk_xin", "clk_ahb";
+ clocks = <&scmi_clk KEEM_BAY_PSS_AUX_SD1>,
+ <&scmi_clk KEEM_BAY_PSS_SD1>;
+ arasan,soc-ctl-syscon = <&sd1_phy_syscon>;
+ };
--
2.17.1
^ permalink raw reply related
* [PATCH v2 0/3] mmc: sdhci-of-arasan: Add support for Intel Keem Bay
From: Wan Ahmad Zainie @ 2020-05-26 6:27 UTC (permalink / raw)
To: ulf.hansson, robh+dt, adrian.hunter, michal.simek
Cc: linux-mmc, devicetree, wan.ahmad.zainie.wan.mohamad
Hi.
The first part is to document the new compatible string required
for eMMC, SD and SDIO support in Intel Keem Bay.
The second part is to add the changes required to support the Host
Controller and its integrated eMMC PHY. The significant change is
to disable 64-bit support.
The patch was tested with Keem Bay evaluation module board.
Thank you.
Best regards,
Zainie
Changes since v1:
- Add patch to convert arasan,sdhci.txt to yaml format.
Wan Ahmad Zainie (3):
dt-bindings: mmc: arasan: Add compatible strings for Intel Keem Bay
mmc: sdhci-of-arasan: Add support for Intel Keem Bay
dt-bindings: mmc: convert arasan sdhci bindings to yaml
.../devicetree/bindings/mmc/arasan,sdhci.txt | 150 ---------
.../devicetree/bindings/mmc/arasan,sdhci.yaml | 293 ++++++++++++++++++
drivers/mmc/host/sdhci-of-arasan.c | 123 ++++++++
3 files changed, 416 insertions(+), 150 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
create mode 100644 Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
--
2.17.1
^ permalink raw reply
* Re: [PATCH 3/3] hwrng: ba431-rng: add support for BA431 hwrng
From: Olivier Sobrie @ 2020-05-26 5:16 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Matt Mackall, Herbert Xu, Rob Herring, Greg Kroah-Hartman,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, DTML,
linux-kernel@vger.kernel.org, Waleed Ziad, sebastien.rabou
In-Reply-To: <CAK8P3a3=HoQZuBoqyFgyde1X7BRfcH-GFQpu=8acOi_JhVU99g@mail.gmail.com>
On Mon, May 25, 2020 at 10:28:46PM +0200, Arnd Bergmann wrote:
> On Mon, May 25, 2020 at 10:07 PM Olivier Sobrie
> <olivier.sobrie@silexinsight.com> wrote:
> >
> > Silex insight BA431 is an IP designed to generate random numbers that
> > can be integrated in various FPGA.
> > This driver adds support for it through the hwrng interface.
> >
> > This driver is used in Silex Insight Viper OEM boards.
> >
> > Signed-off-by: Olivier Sobrie <olivier.sobrie@silexinsight.com>
> > Signed-off-by: Waleed Ziad <waleed94ziad@gmail.com>
>
> The driver looks good to me.
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
>
> > drivers/char/hw_random/Kconfig | 10 ++
> > drivers/char/hw_random/Makefile | 1 +
> > drivers/char/hw_random/ba431-rng.c | 240 +++++++++++++++++++++++++++++
>
> I wonder if we should move drivers/char/hw_random to its own top-level drivers
> subsystem outside of drivers/char. It seems to be growing steadily and is larger
> than a lot of other subsystems with currently 34 drivers in there.
>
> Not your problem though.
>
> > + /* Wait until the state changed */
> > + for (i = 0; i < BA431_RESET_READ_STATUS_RETRIES; ++i) {
> > + state = ba431_trng_get_state(ba431);
> > + if (state >= BA431_STATE_STARTUP)
> > + break;
> > +
> > + udelay(BA431_RESET_READ_STATUS_INTERVAL);
> > + }
>
> Looking for something to improve, I noticed that this loop can take over
> a millisecond to time out, and it always runs in non-atomic context.
> It may be better to use usleep_range() than udelay().
Ok I'll change that and send a v2 later this week.
Thank you,
Olivier
^ permalink raw reply
* Re: [PATCH v3 0/3] Add Qualcomm IPCC driver support
From: Manivannan Sadhasivam @ 2020-05-26 5:13 UTC (permalink / raw)
To: jassisinghbrar
Cc: bjorn.andersson, linux-arm-msm, linux-kernel, devicetree, robh+dt
In-Reply-To: <20200520084854.19729-1-manivannan.sadhasivam@linaro.org>
Hi Jassi,
On Wed, May 20, 2020 at 02:18:51PM +0530, Manivannan Sadhasivam wrote:
> Hello,
>
> This series adds mailbox driver support for Qualcomm Inter Processor
> Communications Controller (IPCC) block found in MSM chipsets. This block
> is used to route interrupts between modems, DSPs and APSS (Application
> Processor Subsystem).
>
> The driver is modeled as a mailbox+irqchip driver. The irqchip part helps
> in receiving the interrupts from the IPCC clients such as modems, DSPs,
> PCI-E etc... and forwards them to respective entities in APSS.
>
> On the other hand, the mailbox part is used to send interrupts to the IPCC
> clients from the entities of APSS.
>
> This series is tested on SM8250-MTP board.
>
Any update on this series?
Thanks,
Mani
> Thanks,
> Mani
>
> Changes in v3:
>
> * Added Bjorn's review tags
> * Few changes to DT binding as suggested by Rob
>
> Changes in v2:
>
> * Moved from soc/ to mailbox/
> * Switched to static mbox channels
> * Some misc cleanups
>
> Manivannan Sadhasivam (3):
> dt-bindings: mailbox: Add devicetree binding for Qcom IPCC
> mailbox: Add support for Qualcomm IPCC
> MAINTAINERS: Add entry for Qualcomm IPCC driver
>
> .../bindings/mailbox/qcom-ipcc.yaml | 80 +++++
> MAINTAINERS | 8 +
> drivers/mailbox/Kconfig | 10 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/qcom-ipcc.c | 286 ++++++++++++++++++
> include/dt-bindings/mailbox/qcom-ipcc.h | 33 ++
> 6 files changed, 419 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/qcom-ipcc.yaml
> create mode 100644 drivers/mailbox/qcom-ipcc.c
> create mode 100644 include/dt-bindings/mailbox/qcom-ipcc.h
>
> --
> 2.26.GIT
>
^ permalink raw reply
* [PATCH v2 1/2] dt-bindings: phy: intel: Add Keem Bay eMMC PHY bindings
From: Wan Ahmad Zainie @ 2020-05-26 5:04 UTC (permalink / raw)
To: kishon, vkoul, robh+dt
Cc: linux-kernel, devicetree, andriy.shevchenko, adrian.hunter,
wan.ahmad.zainie.wan.mohamad
In-Reply-To: <20200526050452.8837-1-wan.ahmad.zainie.wan.mohamad@intel.com>
Binding description for Intel Keem Bay eMMC PHY.
Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
---
.../bindings/phy/intel,keembay-emmc-phy.yaml | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/intel,keembay-emmc-phy.yaml
diff --git a/Documentation/devicetree/bindings/phy/intel,keembay-emmc-phy.yaml b/Documentation/devicetree/bindings/phy/intel,keembay-emmc-phy.yaml
new file mode 100644
index 000000000000..d3e0f169eb0a
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/intel,keembay-emmc-phy.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020 Intel Corporation
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/phy/intel,keembay-emmc-phy.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Intel Keem Bay eMMC PHY bindings
+
+maintainers:
+ - Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
+
+properties:
+ compatible:
+ const: intel,keembay-emmc-phy
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: emmcclk
+
+ "#phy-cells":
+ const: 0
+
+required:
+ - compatible
+ - reg
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ phy@20290000 {
+ compatible = "intel,keembay-emmc-phy";
+ reg = <0x0 0x20290000 0x0 0x54>;
+ clocks = <&emmc>;
+ clock-names = "emmcclk";
+ #phy-cells = <0>;
+ };
--
2.17.1
^ permalink raw reply related
* [PATCH 2/2] arm64: tegra: Add pwm-fan profile settings
From: Sandipan Patra @ 2020-05-26 5:06 UTC (permalink / raw)
To: treding, jonathanh, u.kleine-koenig, kamil, jdelvare, linux,
robh+dt
Cc: bbasu, bbiswas, linux-pwm, linux-hwmon, devicetree, linux-tegra,
linux-kernel, Sandipan Patra
In-Reply-To: <1590469565-14953-1-git-send-email-spatra@nvidia.com>
Add support for profiles in device tree to allow
different fan settings for trip point temp/hyst/pwm.
Signed-off-by: Sandipan Patra <spatra@nvidia.com>
---
arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
index e15d1ea..ff2b980 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-0000.dts
@@ -219,10 +219,19 @@
fan: fan {
compatible = "pwm-fan";
- pwms = <&pwm4 0 45334>;
-
- cooling-levels = <0 64 128 255>;
#cooling-cells = <2>;
+ pwms = <&pwm4 0 45334>;
+ profiles {
+ default = "quiet";
+ quiet {
+ state_cap = <4>;
+ cooling-levels = <0 77 120 160 255 255 255 255 255 255>;
+ };
+ cool {
+ state_cap = <4>;
+ cooling-levels = <0 77 120 160 255 255 255 255 255 255>;
+ };
+ };
};
gpio-keys {
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/2] phy: intel: Add Keem Bay eMMC PHY support
From: Wan Ahmad Zainie @ 2020-05-26 5:04 UTC (permalink / raw)
To: kishon, vkoul, robh+dt
Cc: linux-kernel, devicetree, andriy.shevchenko, adrian.hunter,
wan.ahmad.zainie.wan.mohamad
In-Reply-To: <20200526050452.8837-1-wan.ahmad.zainie.wan.mohamad@intel.com>
Add support for eMMC PHY on Intel Keem Bay SoC.
Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>
---
drivers/phy/intel/Kconfig | 8 +
drivers/phy/intel/Makefile | 1 +
drivers/phy/intel/phy-keembay-emmc.c | 321 +++++++++++++++++++++++++++
3 files changed, 330 insertions(+)
create mode 100644 drivers/phy/intel/phy-keembay-emmc.c
diff --git a/drivers/phy/intel/Kconfig b/drivers/phy/intel/Kconfig
index 7b47682a4e0e..5f5497d1624a 100644
--- a/drivers/phy/intel/Kconfig
+++ b/drivers/phy/intel/Kconfig
@@ -22,3 +22,11 @@ config PHY_INTEL_EMMC
select GENERIC_PHY
help
Enable this to support the Intel EMMC PHY
+
+config PHY_KEEMBAY_EMMC
+ tristate "Intel Keem Bay EMMC PHY Driver"
+ depends on OF
+ select GENERIC_PHY
+ select REGMAP_MMIO
+ help
+ Enable this to support the Keem Bay EMMC PHY.
diff --git a/drivers/phy/intel/Makefile b/drivers/phy/intel/Makefile
index 233d530dadde..6566334e7b77 100644
--- a/drivers/phy/intel/Makefile
+++ b/drivers/phy/intel/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_PHY_INTEL_COMBO) += phy-intel-combo.o
obj-$(CONFIG_PHY_INTEL_EMMC) += phy-intel-emmc.o
+obj-$(CONFIG_PHY_KEEMBAY_EMMC) += phy-keembay-emmc.o
diff --git a/drivers/phy/intel/phy-keembay-emmc.c b/drivers/phy/intel/phy-keembay-emmc.c
new file mode 100644
index 000000000000..546854cdbb0c
--- /dev/null
+++ b/drivers/phy/intel/phy-keembay-emmc.c
@@ -0,0 +1,321 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel Keem Bay eMMC PHY driver
+ * Copyright (C) 2020 Intel Corporation
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+/* eMMC/SD/SDIO core/phy configuration registers */
+#define PHY_CFG_0 0x24
+#define SEL_DLY_TXCLK_MASK BIT(29)
+#define SEL_DLY_TXCLK(x) (((x) << 29) & SEL_DLY_TXCLK_MASK)
+#define OTAP_DLY_ENA_MASK BIT(27)
+#define OTAP_DLY_ENA(x) (((x) << 27) & OTAP_DLY_ENA_MASK)
+#define OTAP_DLY_SEL_MASK GENMASK(26, 23)
+#define OTAP_DLY_SEL(x) (((x) << 23) & OTAP_DLY_SEL_MASK)
+#define DLL_EN_MASK BIT(10)
+#define DLL_EN(x) (((x) << 10) & DLL_EN_MASK)
+#define PWR_DOWN_MASK BIT(0)
+#define PWR_DOWN(x) (((x) << 0) & PWR_DOWN_MASK)
+
+#define PHY_CFG_2 0x2c
+#define SEL_FREQ_MASK GENMASK(12, 10)
+#define SEL_FREQ(x) (((x) << 10) & SEL_FREQ_MASK)
+
+#define PHY_STAT 0x40
+#define CAL_DONE_MASK BIT(6)
+#define IS_CALDONE(x) ((x) & CAL_DONE_MASK)
+#define DLL_RDY_MASK BIT(5)
+#define IS_DLLRDY(x) ((x) & DLL_RDY_MASK)
+
+/* From ACS_eMMC51_16nFFC_RO1100_Userguide_v1p0.pdf p17 */
+#define FREQSEL_200M_170M 0x0
+#define FREQSEL_170M_140M 0x1
+#define FREQSEL_140M_110M 0x2
+#define FREQSEL_110M_80M 0x3
+#define FREQSEL_80M_50M 0x4
+
+struct keembay_emmc_phy {
+ struct regmap *syscfg;
+ struct clk *emmcclk;
+};
+
+static const struct regmap_config keembay_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+};
+
+static int keembay_emmc_phy_power(struct phy *phy, bool on_off)
+{
+ struct keembay_emmc_phy *priv = phy_get_drvdata(phy);
+ unsigned int caldone;
+ unsigned int dllrdy;
+ unsigned int freqsel;
+ unsigned int mhz;
+ int ret;
+
+ /*
+ * Keep phyctrl_pdb and phyctrl_endll low to allow
+ * initialization of CALIO state M/C DFFs
+ */
+ ret = regmap_update_bits(priv->syscfg, PHY_CFG_0, PWR_DOWN_MASK,
+ PWR_DOWN(0));
+ if (ret) {
+ dev_err(&phy->dev, "CALIO power down bar failed: %d\n", ret);
+ return ret;
+ }
+
+ ret = regmap_update_bits(priv->syscfg, PHY_CFG_0, DLL_EN_MASK,
+ DLL_EN(0));
+ if (ret) {
+ dev_err(&phy->dev, "turn off the dll failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Already finish power off above */
+ if (!on_off)
+ return 0;
+
+ mhz = DIV_ROUND_CLOSEST(clk_get_rate(priv->emmcclk), 1000000);
+ if (mhz <= 200 && mhz >= 170)
+ freqsel = FREQSEL_200M_170M;
+ else if (mhz <= 170 && mhz >= 140)
+ freqsel = FREQSEL_170M_140M;
+ else if (mhz <= 140 && mhz >= 110)
+ freqsel = FREQSEL_140M_110M;
+ else if (mhz <= 110 && mhz >= 80)
+ freqsel = FREQSEL_110M_80M;
+ else if (mhz <= 80 && mhz >= 50)
+ freqsel = FREQSEL_80M_50M;
+ else
+ freqsel = 0x0;
+
+ if (mhz < 50 || mhz > 200)
+ dev_warn(&phy->dev, "Unsupported rate: %d MHz\n", mhz);
+
+ /*
+ * According to the user manual, calpad calibration
+ * cycle takes more than 2us without the minimal recommended
+ * value, so we may need a little margin here
+ */
+ udelay(5);
+
+ ret = regmap_update_bits(priv->syscfg, PHY_CFG_0, PWR_DOWN_MASK,
+ PWR_DOWN(1));
+ if (ret) {
+ dev_err(&phy->dev, "CALIO power down bar failed: %d\n", ret);
+ return ret;
+ }
+
+ /*
+ * According to the user manual, it asks driver to wait 5us for
+ * calpad busy trimming. However it is documented that this value is
+ * PVT(A.K.A. process, voltage and temperature) relevant, so some
+ * failure cases are found which indicates we should be more tolerant
+ * to calpad busy trimming.
+ */
+ ret = regmap_read_poll_timeout(priv->syscfg, PHY_STAT,
+ caldone, IS_CALDONE(caldone),
+ 0, 50);
+ if (ret) {
+ dev_err(&phy->dev, "caldone failed, ret=%d\n", ret);
+ return ret;
+ }
+
+ /* Set the frequency of the DLL operation */
+ ret = regmap_update_bits(priv->syscfg, PHY_CFG_2, SEL_FREQ_MASK,
+ SEL_FREQ(freqsel));
+ if (ret) {
+ dev_err(&phy->dev, "set the frequency of dll failed:%d\n", ret);
+ return ret;
+ }
+
+ /* Turn on the DLL */
+ ret = regmap_update_bits(priv->syscfg, PHY_CFG_0, DLL_EN_MASK,
+ DLL_EN(1));
+ if (ret) {
+ dev_err(&phy->dev, "turn on the dll failed: %d\n", ret);
+ return ret;
+ }
+
+ /*
+ * We turned on the DLL even though the rate was 0 because we the
+ * clock might be turned on later. ...but we can't wait for the DLL
+ * to lock when the rate is 0 because it will never lock with no
+ * input clock.
+ *
+ * Technically we should be checking the lock later when the clock
+ * is turned on, but for now we won't.
+ */
+ if (mhz == 0)
+ return 0;
+
+ /*
+ * After enabling analog DLL circuits docs say that we need 10.2 us if
+ * our source clock is at 50 MHz and that lock time scales linearly
+ * with clock speed. If we are powering on the PHY and the card clock
+ * is super slow (like 100kHz) this could take as long as 5.1 ms as
+ * per the math: 10.2 us * (50000000 Hz / 100000 Hz) => 5.1 ms
+ * hopefully we won't be running at 100 kHz, but we should still make
+ * sure we wait long enough.
+ *
+ * NOTE: There appear to be corner cases where the DLL seems to take
+ * extra long to lock for reasons that aren't understood. In some
+ * extreme cases we've seen it take up to over 10ms (!). We'll be
+ * generous and give it 50ms.
+ */
+ ret = regmap_read_poll_timeout(priv->syscfg, PHY_STAT,
+ dllrdy, IS_DLLRDY(dllrdy),
+ 0, 50 * USEC_PER_MSEC);
+ if (ret) {
+ dev_err(&phy->dev, "dllrdy failed, ret=%d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int keembay_emmc_phy_init(struct phy *phy)
+{
+ struct keembay_emmc_phy *priv = phy_get_drvdata(phy);
+
+ /*
+ * We purposely get the clock here and not in probe to avoid the
+ * circular dependency problem. We expect:
+ * - PHY driver to probe
+ * - SDHCI driver to start probe
+ * - SDHCI driver to register it's clock
+ * - SDHCI driver to get the PHY
+ * - SDHCI driver to init the PHY
+ *
+ * The clock is optional, so upon any error just return it like
+ * any other error to user.
+ */
+ priv->emmcclk = clk_get_optional(&phy->dev, "emmcclk");
+ if (IS_ERR(priv->emmcclk)) {
+ dev_err(&phy->dev, "ERROR: getting emmcclk\n");
+ return PTR_ERR(priv->emmcclk);
+ }
+
+ return 0;
+}
+
+static int keembay_emmc_phy_exit(struct phy *phy)
+{
+ struct keembay_emmc_phy *priv = phy_get_drvdata(phy);
+
+ clk_put(priv->emmcclk);
+
+ return 0;
+};
+
+static int keembay_emmc_phy_power_on(struct phy *phy)
+{
+ struct keembay_emmc_phy *priv = phy_get_drvdata(phy);
+ int ret;
+
+ /* Delay chain based txclk: enable */
+ ret = regmap_update_bits(priv->syscfg, PHY_CFG_0, SEL_DLY_TXCLK_MASK,
+ SEL_DLY_TXCLK(1));
+ if (ret) {
+ dev_err(&phy->dev, "ERROR: delay chain txclk set: %d\n", ret);
+ return ret;
+ }
+
+ /* Output tap delay: enable */
+ ret = regmap_update_bits(priv->syscfg, PHY_CFG_0, OTAP_DLY_ENA_MASK,
+ OTAP_DLY_ENA(1));
+ if (ret) {
+ dev_err(&phy->dev, "ERROR: output tap delay set: %d\n", ret);
+ return ret;
+ }
+
+ /* Output tap delay */
+ ret = regmap_update_bits(priv->syscfg, PHY_CFG_0, OTAP_DLY_SEL_MASK,
+ OTAP_DLY_SEL(2));
+ if (ret) {
+ dev_err(&phy->dev, "ERROR: output tap delay select: %d\n", ret);
+ return ret;
+ }
+
+ /* Power up eMMC phy analog blocks */
+ return keembay_emmc_phy_power(phy, true);
+}
+
+static int keembay_emmc_phy_power_off(struct phy *phy)
+{
+ /* Power down eMMC phy analog blocks */
+ return keembay_emmc_phy_power(phy, false);
+}
+
+static const struct phy_ops ops = {
+ .init = keembay_emmc_phy_init,
+ .exit = keembay_emmc_phy_exit,
+ .power_on = keembay_emmc_phy_power_on,
+ .power_off = keembay_emmc_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int keembay_emmc_phy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct keembay_emmc_phy *priv;
+ struct phy *generic_phy;
+ struct phy_provider *phy_provider;
+ void __iomem *base;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ priv->syscfg = devm_regmap_init_mmio(dev, base,
+ &keembay_regmap_config);
+ if (IS_ERR(priv->syscfg))
+ return PTR_ERR(priv->syscfg);
+
+ generic_phy = devm_phy_create(dev, np, &ops);
+ if (IS_ERR(generic_phy)) {
+ dev_err(dev, "failed to create PHY\n");
+ return PTR_ERR(generic_phy);
+ }
+
+ phy_set_drvdata(generic_phy, priv);
+ phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id keembay_emmc_phy_dt_ids[] = {
+ { .compatible = "intel,keembay-emmc-phy" },
+ {}
+};
+
+MODULE_DEVICE_TABLE(of, keembay_emmc_phy_dt_ids);
+
+static struct platform_driver keembay_emmc_phy_driver = {
+ .probe = keembay_emmc_phy_probe,
+ .driver = {
+ .name = "keembay-emmc-phy",
+ .of_match_table = keembay_emmc_phy_dt_ids,
+ },
+};
+module_platform_driver(keembay_emmc_phy_driver);
+
+MODULE_AUTHOR("Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>");
+MODULE_DESCRIPTION("Intel Keem Bay eMMC PHY driver");
+MODULE_LICENSE("GPL v2");
--
2.17.1
^ permalink raw reply related
* [PATCH v2 0/2] phy: intel: Add Keem Bay eMMC PHY support
From: Wan Ahmad Zainie @ 2020-05-26 5:04 UTC (permalink / raw)
To: kishon, vkoul, robh+dt
Cc: linux-kernel, devicetree, andriy.shevchenko, adrian.hunter,
wan.ahmad.zainie.wan.mohamad
Hi.
The first part is to document DT bindings for Keem Bay eMMC PHY.
The second is the driver file, loosely based on phy-rockchip-emmc.c
and phy-intel-emmc.c. The latter is not being reused as there are
quite a number of differences i.e. registers offset, supported clock
rates, bitfield to set.
The patch was tested with Keem Bay evaluation module board.
Thank you.
Best regards,
Zainie
Changes since v1:
- Rework phy-keembay-emmc.c to make it similar to phy-intel-emmc.c.
- Use regmap_mmio, and remove reference to intel,syscon.
- Use node name phy@....
- Update license i.e. use dual license.
Wan Ahmad Zainie (2):
dt-bindings: phy: intel: Add Keem Bay eMMC PHY bindings
phy: intel: Add Keem Bay eMMC PHY support
.../bindings/phy/intel,keembay-emmc-phy.yaml | 45 +++
drivers/phy/intel/Kconfig | 8 +
drivers/phy/intel/Makefile | 1 +
drivers/phy/intel/phy-keembay-emmc.c | 321 ++++++++++++++++++
4 files changed, 375 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/intel,keembay-emmc-phy.yaml
create mode 100644 drivers/phy/intel/phy-keembay-emmc.c
--
2.17.1
^ permalink raw reply
* [PATCH 1/2] hwmon: pwm-fan: Add profile support and add remove module support
From: Sandipan Patra @ 2020-05-26 5:06 UTC (permalink / raw)
To: treding, jonathanh, u.kleine-koenig, kamil, jdelvare, linux,
robh+dt
Cc: bbasu, bbiswas, linux-pwm, linux-hwmon, devicetree, linux-tegra,
linux-kernel, Sandipan Patra
This change has 2 parts:
1. Add support for profiles mode settings.
This allows different fan settings for trip point temp/hyst/pwm.
T194 has multiple fan-profiles support.
2. Add pwm-fan remove support. This is essential since the config is
tristate capable.
Signed-off-by: Sandipan Patra <spatra@nvidia.com>
---
drivers/hwmon/pwm-fan.c | 112 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 100 insertions(+), 12 deletions(-)
diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 30b7b3e..26db589 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -3,8 +3,10 @@
* pwm-fan.c - Hwmon driver for fans connected to PWM lines.
*
* Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2020, NVIDIA Corporation.
*
* Author: Kamil Debski <k.debski@samsung.com>
+ * Author: Sandipan Patra <spatra@nvidia.com>
*/
#include <linux/hwmon.h>
@@ -21,6 +23,8 @@
#include <linux/timer.h>
#define MAX_PWM 255
+/* Based on OF max device tree node name length */
+#define MAX_PROFILE_NAME_LENGTH 31
struct pwm_fan_ctx {
struct mutex lock;
@@ -38,6 +42,12 @@ struct pwm_fan_ctx {
unsigned int pwm_fan_state;
unsigned int pwm_fan_max_state;
unsigned int *pwm_fan_cooling_levels;
+
+ unsigned int pwm_fan_profiles;
+ const char **fan_profile_names;
+ unsigned int **fan_profile_cooling_levels;
+ unsigned int fan_current_profile;
+
struct thermal_cooling_device *cdev;
};
@@ -227,28 +237,86 @@ static int pwm_fan_of_get_cooling_data(struct device *dev,
struct pwm_fan_ctx *ctx)
{
struct device_node *np = dev->of_node;
+ struct device_node *base_profile = NULL;
+ struct device_node *profile_np = NULL;
+ const char *default_profile = NULL;
int num, i, ret;
- if (!of_find_property(np, "cooling-levels", NULL))
- return 0;
+ num = of_property_count_u32_elems(np, "cooling-levels");
+ if (num <= 0) {
+ base_profile = of_get_child_by_name(np, "profiles");
+ if (!base_profile) {
+ dev_err(dev, "Wrong Data\n");
+ return -EINVAL;
+ }
+ }
+
+ if (base_profile) {
+ ctx->pwm_fan_profiles =
+ of_get_available_child_count(base_profile);
+
+ if (ctx->pwm_fan_profiles <= 0) {
+ dev_err(dev, "Profiles used but not defined\n");
+ return -EINVAL;
+ }
- ret = of_property_count_u32_elems(np, "cooling-levels");
- if (ret <= 0) {
- dev_err(dev, "Wrong data!\n");
- return ret ? : -EINVAL;
+ ctx->fan_profile_names = devm_kzalloc(dev,
+ sizeof(const char *) * ctx->pwm_fan_profiles,
+ GFP_KERNEL);
+ ctx->fan_profile_cooling_levels = devm_kzalloc(dev,
+ sizeof(int *) * ctx->pwm_fan_profiles,
+ GFP_KERNEL);
+
+ if (!ctx->fan_profile_names
+ || !ctx->fan_profile_cooling_levels)
+ return -ENOMEM;
+
+ ctx->fan_current_profile = 0;
+ i = 0;
+ for_each_available_child_of_node(base_profile, profile_np) {
+ num = of_property_count_u32_elems(profile_np,
+ "cooling-levels");
+ if (num <= 0) {
+ dev_err(dev, "No data in cooling-levels inside profile node!\n");
+ return -EINVAL;
+ }
+
+ of_property_read_string(profile_np, "name",
+ &ctx->fan_profile_names[i]);
+ if (default_profile &&
+ !strncmp(default_profile,
+ ctx->fan_profile_names[i],
+ MAX_PROFILE_NAME_LENGTH))
+ ctx->fan_current_profile = i;
+
+ ctx->fan_profile_cooling_levels[i] =
+ devm_kzalloc(dev, sizeof(int) * num,
+ GFP_KERNEL);
+ if (!ctx->fan_profile_cooling_levels[i])
+ return -ENOMEM;
+
+ of_property_read_u32_array(profile_np, "cooling-levels",
+ ctx->fan_profile_cooling_levels[i], num);
+ i++;
+ }
}
- num = ret;
ctx->pwm_fan_cooling_levels = devm_kcalloc(dev, num, sizeof(u32),
GFP_KERNEL);
if (!ctx->pwm_fan_cooling_levels)
return -ENOMEM;
- ret = of_property_read_u32_array(np, "cooling-levels",
- ctx->pwm_fan_cooling_levels, num);
- if (ret) {
- dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
- return ret;
+ if (base_profile) {
+ memcpy(ctx->pwm_fan_cooling_levels,
+ ctx->fan_profile_cooling_levels[ctx->fan_current_profile],
+ num);
+ } else {
+ ret = of_property_read_u32_array(np, "cooling-levels",
+ ctx->pwm_fan_cooling_levels, num);
+ if (ret) {
+ dev_err(dev, "Property 'cooling-levels' cannot be read!\n");
+ return -EINVAL;
+ }
}
for (i = 0; i < num; i++) {
@@ -390,6 +458,25 @@ static int pwm_fan_probe(struct platform_device *pdev)
return 0;
}
+static int pwm_fan_remove(struct platform_device *pdev)
+{
+ struct pwm_fan_ctx *ctx = platform_get_drvdata(pdev);
+ struct pwm_args args;
+
+ if (!ctx)
+ return -EINVAL;
+
+ if (IS_ENABLED(CONFIG_THERMAL))
+ thermal_cooling_device_unregister(ctx->cdev);
+
+ pwm_get_args(ctx->pwm, &args);
+ pwm_config(ctx->pwm, 0, args.period);
+ pwm_disable(ctx->pwm);
+
+ return 0;
+}
+
+
static int pwm_fan_disable(struct device *dev)
{
struct pwm_fan_ctx *ctx = dev_get_drvdata(dev);
@@ -465,6 +552,7 @@ MODULE_DEVICE_TABLE(of, of_pwm_fan_match);
static struct platform_driver pwm_fan_driver = {
.probe = pwm_fan_probe,
+ .remove = pwm_fan_remove,
.shutdown = pwm_fan_shutdown,
.driver = {
.name = "pwm-fan",
--
2.7.4
^ permalink raw reply related
* RE: [EXT] Re: [PATCH] arm64: dts: ls1028a: add one more thermal zone support
From: Andy Tang @ 2020-05-26 2:45 UTC (permalink / raw)
To: Daniel Lezcano, shawnguo@kernel.org, robh+dt@kernel.org,
mark.rutland@arm.com, catalin.marinas@arm.com,
will.deacon@arm.com
Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <bdfd7018-aed8-ddbe-8bd2-2fa834013218@linaro.org>
-----Original Message-----
From: Daniel Lezcano <daniel.lezcano@linaro.org>
Sent: 2020年5月25日 19:08
To: Andy Tang <andy.tang@nxp.com>; shawnguo@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; catalin.marinas@arm.com; will.deacon@arm.com
Cc: devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Subject: [EXT] Re: [PATCH] arm64: dts: ls1028a: add one more thermal zone support
Caution: EXT Email
On 25/05/2020 09:38, Yuantian Tang wrote:
> There are 2 thermal zones in ls1028a soc. Current dts only includes
> one. This patch adds the other thermal zone node in dts to enable it.
For my personal information, is there a cooling device for the DDR?
A: There is only one cooling device which is used by core-cluster sensor zone.
So there is no cooling device for DDR.
BR,
Andy
> Signed-off-by: Yuantian Tang <andy.tang@nxp.com>
> ---
> .../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 22
> ++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> index 055f114cf848..bc6f0c0f85da 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> @@ -129,11 +129,31 @@
> };
>
> thermal-zones {
> - core-cluster {
> + ddr-controller {
> polling-delay-passive = <1000>;
> polling-delay = <5000>;
> thermal-sensors = <&tmu 0>;
>
> + trips {
> + ddr-ctrler-alert {
> + temperature = <85000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> +
> + ddr-ctrler-crit {
> + temperature = <95000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + core-cluster {
> + polling-delay-passive = <1000>;
> + polling-delay = <5000>;
> + thermal-sensors = <&tmu 1>;
> +
> trips {
> core_cluster_alert: core-cluster-alert {
> temperature = <85000>;
>
--
<https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.linaro.org%2F&data=02%7C01%7Candy.tang%40nxp.com%7Cfb12ed47db3c4912bf1b08d8009be7d0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637260016892509132&sdata=WUnbHs3O1yLfoRZ2usPXAu7EGBoSL7vypnmrhTGHaGI%3D&reserved=0> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.facebook.com%2Fpages%2FLinaro&data=02%7C01%7Candy.tang%40nxp.com%7Cfb12ed47db3c4912bf1b08d8009be7d0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637260016892509132&sdata=W4k8Vs3wVVvfeGN4xVxKgBl3mG2j%2FjMJj07NEAqpAck%3D&reserved=0> Facebook | <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitter.com%2F%23!%2Flinaroorg&data=02%7C01%7Candy.tang%40nxp.com%7Cfb12ed47db3c4912bf1b08d8009be7d0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637260016892509132&sdata=OeFLPA0lZHuvGkm1OuT38PxAUpLMsr7fBXEBkEGWXc8%3D&reserved=0> Twitter | <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.linaro.org%2Flinaro-blog%2F&data=02%7C01%7Candy.tang%40nxp.com%7Cfb12ed47db3c4912bf1b08d8009be7d0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637260016892509132&sdata=aYR%2FaUzcFxsMDG9QaH0Ge7AKg0kJniSOSPE6fXoJpzw%3D&reserved=0> Blog
^ permalink raw reply
* Re: [PATCH v8 0/5] support reserving crashkernel above 4G on arm64 kdump
From: chenzhou @ 2020-05-26 2:28 UTC (permalink / raw)
To: Baoquan He
Cc: tglx, mingo, catalin.marinas, will, dyoung, robh+dt,
John.p.donnelly, arnd, devicetree, linux-doc, kexec, linux-kernel,
horms, guohanjun, pkushwaha, linux-arm-kernel
In-Reply-To: <20200526014242.GF20045@MiWiFi-R3L-srv>
Hi Baoquan,
Thanks for your suggestions.
You are right, some details should be made in the commit log.
Thanks,
Chen Zhou
On 2020/5/26 9:42, Baoquan He wrote:
> On 05/21/20 at 05:38pm, Chen Zhou wrote:
>> This patch series enable reserving crashkernel above 4G in arm64.
>>
>> There are following issues in arm64 kdump:
>> 1. We use crashkernel=X to reserve crashkernel below 4G, which will fail
>> when there is no enough low memory.
>> 2. Currently, crashkernel=Y@X can be used to reserve crashkernel above 4G,
>> in this case, if swiotlb or DMA buffers are required, crash dump kernel
>> will boot failure because there is no low memory available for allocation.
>>
>> To solve these issues, introduce crashkernel=X,low to reserve specified
>> size low memory.
>> Crashkernel=X tries to reserve memory for the crash dump kernel under
>> 4G. If crashkernel=Y,low is specified simultaneously, reserve spcified
>> size low memory for crash kdump kernel devices firstly and then reserve
>> memory above 4G.
>>
>> When crashkernel is reserved above 4G in memory, that is, crashkernel=X,low
>> is specified simultaneously, kernel should reserve specified size low memory
>> for crash dump kernel devices. So there may be two crash kernel regions, one
>> is below 4G, the other is above 4G.
>> In order to distinct from the high region and make no effect to the use of
>> kexec-tools, rename the low region as "Crash kernel (low)", and add DT property
>> "linux,low-memory-range" to crash dump kernel's dtb to pass the low region.
>>
>> Besides, we need to modify kexec-tools:
>> arm64: kdump: add another DT property to crash dump kernel's dtb(see [1])
>>
>> The previous changes and discussions can be retrieved from:
>>
>> Changes since [v7]
>> - Move x86 CRASH_ALIGN to 2M
>> Suggested by Dave and do some test, move x86 CRASH_ALIGN to 2M.
> OK, moving x86 CRASH_ALIGN to 2M is suggested by Dave. Because
> CONFIG_PHYSICAL_ALIGN can be selected from 2M to 16M. So 2M seems good.
> But, anyway, we should tell the reason why it need be changed in commit
> log.
>
>
> arch/x86/Kconfig:
> config PHYSICAL_ALIGN
> hex "Alignment value to which kernel should be aligned"
> default "0x200000"
> range 0x2000 0x1000000 if X86_32
> range 0x200000 0x1000000 if X86_64
>
>> - Update Documentation/devicetree/bindings/chosen.txt
>> Add corresponding documentation to Documentation/devicetree/bindings/chosen.txt suggested by Arnd.
>> - Add Tested-by from Jhon and pk
>>
>> Changes since [v6]
>> - Fix build errors reported by kbuild test robot.
>>
>> Changes since [v5]
>> - Move reserve_crashkernel_low() into kernel/crash_core.c.
>> - Delete crashkernel=X,high.
> And the crashkernel=X,high being deleted need be told too. Otherwise
> people reading the commit have to check why themselves. I didn't follow
> the old version, can't see why ,high can't be specified explicitly.
>
>> - Modify crashkernel=X,low.
>> If crashkernel=X,low is specified simultaneously, reserve spcified size low
>> memory for crash kdump kernel devices firstly and then reserve memory above 4G.
>> In addition, rename crashk_low_res as "Crash kernel (low)" for arm64, and then
>> pass to crash dump kernel by DT property "linux,low-memory-range".
>> - Update Documentation/admin-guide/kdump/kdump.rst.
>>
>> Changes since [v4]
>> - Reimplement memblock_cap_memory_ranges for multiple ranges by Mike.
>>
>> Changes since [v3]
>> - Add memblock_cap_memory_ranges back for multiple ranges.
>> - Fix some compiling warnings.
>>
>> Changes since [v2]
>> - Split patch "arm64: kdump: support reserving crashkernel above 4G" as
>> two. Put "move reserve_crashkernel_low() into kexec_core.c" in a separate
>> patch.
>>
>> Changes since [v1]:
>> - Move common reserve_crashkernel_low() code into kernel/kexec_core.c.
>> - Remove memblock_cap_memory_ranges() i added in v1 and implement that
>> in fdt_enforce_memory_region().
>> There are at most two crash kernel regions, for two crash kernel regions
>> case, we cap the memory range [min(regs[*].start), max(regs[*].end)]
>> and then remove the memory range in the middle.
>>
>> [1]: http://lists.infradead.org/pipermail/kexec/2020-May/025128.html
>> [v1]: https://lkml.org/lkml/2019/4/2/1174
>> [v2]: https://lkml.org/lkml/2019/4/9/86
>> [v3]: https://lkml.org/lkml/2019/4/9/306
>> [v4]: https://lkml.org/lkml/2019/4/15/273
>> [v5]: https://lkml.org/lkml/2019/5/6/1360
>> [v6]: https://lkml.org/lkml/2019/8/30/142
>> [v7]: https://lkml.org/lkml/2019/12/23/411
>>
>> Chen Zhou (5):
>> x86: kdump: move reserve_crashkernel_low() into crash_core.c
>> arm64: kdump: reserve crashkenel above 4G for crash dump kernel
>> arm64: kdump: add memory for devices by DT property, low-memory-range
>> kdump: update Documentation about crashkernel on arm64
>> dt-bindings: chosen: Document linux,low-memory-range for arm64 kdump
>>
>> Documentation/admin-guide/kdump/kdump.rst | 13 ++-
>> .../admin-guide/kernel-parameters.txt | 12 ++-
>> Documentation/devicetree/bindings/chosen.txt | 25 ++++++
>> arch/arm64/kernel/setup.c | 8 +-
>> arch/arm64/mm/init.c | 61 ++++++++++++-
>> arch/x86/kernel/setup.c | 66 ++------------
>> include/linux/crash_core.h | 3 +
>> include/linux/kexec.h | 2 -
>> kernel/crash_core.c | 85 +++++++++++++++++++
>> kernel/kexec_core.c | 17 ----
>> 10 files changed, 208 insertions(+), 84 deletions(-)
>>
>> --
>> 2.20.1
>>
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec
>>
>
> .
>
^ permalink raw reply
* Re: [PATCH v2 6/6] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml
From: Laurent Pinchart @ 2020-05-26 1:44 UTC (permalink / raw)
To: Ricardo Cañuelo
Cc: kernel, devicetree, linux-arm-kernel, geert+renesas, robh+dt,
xuwei5
In-Reply-To: <20200525074335.grnjvdjnipq5g3kf@rcn-XPS-13-9360>
Hi Ricardo,
On Mon, May 25, 2020 at 09:43:35AM +0200, Ricardo Cañuelo wrote:
> On jue 14-05-2020 18:22:39, Laurent Pinchart wrote:
> > > If we want to be more strict and require the definition of all the
> > > supplies, there will be many more DTs changes in the series, and I'm not
> > > sure I'll be able to do that in a reasonable amount of time. I'm looking
> > > at them and it's not always clear which regulators to use or if they are
> > > even defined.
> >
> > We can decouple the two though (I think). The bindings should reflect
> > what we consider right, and the dts files could be fixed on top.
>
> Do you have a suggestion on how to do this? If we decouple the two
> tasks most of the work would be searching for DTs to fix and finding a
> way to fix each one of them, and unless I do this _before_ the binding
> conversion I'll get a lot of dtbs_check errors.
Rob should answer this question as it will be his decision, but I've
personally never considered non-compliant DT sources to be an obstacle
to bindings conversion to YAML. The DT sources should be fixed, but I
don't see it as a prerequisite (although it's a good practice).
> The binding conversion itself is done, if we go this route the only
> additional change would be to make the supplies required.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v8 0/5] support reserving crashkernel above 4G on arm64 kdump
From: Baoquan He @ 2020-05-26 1:42 UTC (permalink / raw)
To: Chen Zhou
Cc: tglx, mingo, catalin.marinas, will, dyoung, robh+dt,
John.p.donnelly, arnd, devicetree, linux-doc, kexec, linux-kernel,
horms, guohanjun, pkushwaha, linux-arm-kernel
In-Reply-To: <20200521093805.64398-1-chenzhou10@huawei.com>
On 05/21/20 at 05:38pm, Chen Zhou wrote:
> This patch series enable reserving crashkernel above 4G in arm64.
>
> There are following issues in arm64 kdump:
> 1. We use crashkernel=X to reserve crashkernel below 4G, which will fail
> when there is no enough low memory.
> 2. Currently, crashkernel=Y@X can be used to reserve crashkernel above 4G,
> in this case, if swiotlb or DMA buffers are required, crash dump kernel
> will boot failure because there is no low memory available for allocation.
>
> To solve these issues, introduce crashkernel=X,low to reserve specified
> size low memory.
> Crashkernel=X tries to reserve memory for the crash dump kernel under
> 4G. If crashkernel=Y,low is specified simultaneously, reserve spcified
> size low memory for crash kdump kernel devices firstly and then reserve
> memory above 4G.
>
> When crashkernel is reserved above 4G in memory, that is, crashkernel=X,low
> is specified simultaneously, kernel should reserve specified size low memory
> for crash dump kernel devices. So there may be two crash kernel regions, one
> is below 4G, the other is above 4G.
> In order to distinct from the high region and make no effect to the use of
> kexec-tools, rename the low region as "Crash kernel (low)", and add DT property
> "linux,low-memory-range" to crash dump kernel's dtb to pass the low region.
>
> Besides, we need to modify kexec-tools:
> arm64: kdump: add another DT property to crash dump kernel's dtb(see [1])
>
> The previous changes and discussions can be retrieved from:
>
> Changes since [v7]
> - Move x86 CRASH_ALIGN to 2M
> Suggested by Dave and do some test, move x86 CRASH_ALIGN to 2M.
OK, moving x86 CRASH_ALIGN to 2M is suggested by Dave. Because
CONFIG_PHYSICAL_ALIGN can be selected from 2M to 16M. So 2M seems good.
But, anyway, we should tell the reason why it need be changed in commit
log.
arch/x86/Kconfig:
config PHYSICAL_ALIGN
hex "Alignment value to which kernel should be aligned"
default "0x200000"
range 0x2000 0x1000000 if X86_32
range 0x200000 0x1000000 if X86_64
> - Update Documentation/devicetree/bindings/chosen.txt
> Add corresponding documentation to Documentation/devicetree/bindings/chosen.txt suggested by Arnd.
> - Add Tested-by from Jhon and pk
>
> Changes since [v6]
> - Fix build errors reported by kbuild test robot.
>
> Changes since [v5]
> - Move reserve_crashkernel_low() into kernel/crash_core.c.
> - Delete crashkernel=X,high.
And the crashkernel=X,high being deleted need be told too. Otherwise
people reading the commit have to check why themselves. I didn't follow
the old version, can't see why ,high can't be specified explicitly.
> - Modify crashkernel=X,low.
> If crashkernel=X,low is specified simultaneously, reserve spcified size low
> memory for crash kdump kernel devices firstly and then reserve memory above 4G.
> In addition, rename crashk_low_res as "Crash kernel (low)" for arm64, and then
> pass to crash dump kernel by DT property "linux,low-memory-range".
> - Update Documentation/admin-guide/kdump/kdump.rst.
>
> Changes since [v4]
> - Reimplement memblock_cap_memory_ranges for multiple ranges by Mike.
>
> Changes since [v3]
> - Add memblock_cap_memory_ranges back for multiple ranges.
> - Fix some compiling warnings.
>
> Changes since [v2]
> - Split patch "arm64: kdump: support reserving crashkernel above 4G" as
> two. Put "move reserve_crashkernel_low() into kexec_core.c" in a separate
> patch.
>
> Changes since [v1]:
> - Move common reserve_crashkernel_low() code into kernel/kexec_core.c.
> - Remove memblock_cap_memory_ranges() i added in v1 and implement that
> in fdt_enforce_memory_region().
> There are at most two crash kernel regions, for two crash kernel regions
> case, we cap the memory range [min(regs[*].start), max(regs[*].end)]
> and then remove the memory range in the middle.
>
> [1]: http://lists.infradead.org/pipermail/kexec/2020-May/025128.html
> [v1]: https://lkml.org/lkml/2019/4/2/1174
> [v2]: https://lkml.org/lkml/2019/4/9/86
> [v3]: https://lkml.org/lkml/2019/4/9/306
> [v4]: https://lkml.org/lkml/2019/4/15/273
> [v5]: https://lkml.org/lkml/2019/5/6/1360
> [v6]: https://lkml.org/lkml/2019/8/30/142
> [v7]: https://lkml.org/lkml/2019/12/23/411
>
> Chen Zhou (5):
> x86: kdump: move reserve_crashkernel_low() into crash_core.c
> arm64: kdump: reserve crashkenel above 4G for crash dump kernel
> arm64: kdump: add memory for devices by DT property, low-memory-range
> kdump: update Documentation about crashkernel on arm64
> dt-bindings: chosen: Document linux,low-memory-range for arm64 kdump
>
> Documentation/admin-guide/kdump/kdump.rst | 13 ++-
> .../admin-guide/kernel-parameters.txt | 12 ++-
> Documentation/devicetree/bindings/chosen.txt | 25 ++++++
> arch/arm64/kernel/setup.c | 8 +-
> arch/arm64/mm/init.c | 61 ++++++++++++-
> arch/x86/kernel/setup.c | 66 ++------------
> include/linux/crash_core.h | 3 +
> include/linux/kexec.h | 2 -
> kernel/crash_core.c | 85 +++++++++++++++++++
> kernel/kexec_core.c | 17 ----
> 10 files changed, 208 insertions(+), 84 deletions(-)
>
> --
> 2.20.1
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
^ permalink raw reply
* Re: [PATCH v8 2/5] arm64: kdump: reserve crashkenel above 4G for crash dump kernel
From: Baoquan He @ 2020-05-26 0:59 UTC (permalink / raw)
To: Chen Zhou
Cc: tglx, mingo, catalin.marinas, will, dyoung, robh+dt,
John.p.donnelly, arnd, devicetree, linux-doc, kexec, linux-kernel,
horms, guohanjun, pkushwaha, linux-arm-kernel
In-Reply-To: <20200521093805.64398-3-chenzhou10@huawei.com>
On 05/21/20 at 05:38pm, Chen Zhou wrote:
> Crashkernel=X tries to reserve memory for the crash dump kernel under
> 4G. If crashkernel=X,low is specified simultaneously, reserve spcified
> size low memory for crash kdump kernel devices firstly and then reserve
> memory above 4G.
Wondering why crashkernel=,high is not introduced to arm64 to be
consistent with x86_64, to make the behaviour be the same on all
architecutres.
>
> Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
> Tested-by: John Donnelly <John.p.donnelly@oracle.com>
> Tested-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
> ---
> arch/arm64/kernel/setup.c | 8 +++++++-
> arch/arm64/mm/init.c | 31 +++++++++++++++++++++++++++++--
> 2 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 3fd2c11c09fc..a8487e4d3e5a 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -238,7 +238,13 @@ static void __init request_standard_resources(void)
> kernel_data.end <= res->end)
> request_resource(res, &kernel_data);
> #ifdef CONFIG_KEXEC_CORE
> - /* Userspace will find "Crash kernel" region in /proc/iomem. */
> + /*
> + * Userspace will find "Crash kernel" region in /proc/iomem.
> + * Note: the low region is renamed as Crash kernel (low).
> + */
> + if (crashk_low_res.end && crashk_low_res.start >= res->start &&
> + crashk_low_res.end <= res->end)
> + request_resource(res, &crashk_low_res);
> if (crashk_res.end && crashk_res.start >= res->start &&
> crashk_res.end <= res->end)
> request_resource(res, &crashk_res);
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index e42727e3568e..71498acf0cd8 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -81,6 +81,7 @@ static void __init reserve_crashkernel(void)
> {
> unsigned long long crash_base, crash_size;
> int ret;
> + phys_addr_t crash_max = arm64_dma32_phys_limit;
>
> ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
> &crash_size, &crash_base);
> @@ -88,12 +89,38 @@ static void __init reserve_crashkernel(void)
> if (ret || !crash_size)
> return;
>
> + ret = reserve_crashkernel_low();
> + if (!ret && crashk_low_res.end) {
> + /*
> + * If crashkernel=X,low specified, there may be two regions,
> + * we need to make some changes as follows:
> + *
> + * 1. rename the low region as "Crash kernel (low)"
> + * In order to distinct from the high region and make no effect
> + * to the use of existing kexec-tools, rename the low region as
> + * "Crash kernel (low)".
> + *
> + * 2. change the upper bound for crash memory
> + * Set MEMBLOCK_ALLOC_ACCESSIBLE upper bound for crash memory.
> + *
> + * 3. mark the low region as "nomap"
> + * The low region is intended to be used for crash dump kernel
> + * devices, just mark the low region as "nomap" simply.
> + */
> + const char *rename = "Crash kernel (low)";
> +
> + crashk_low_res.name = rename;
> + crash_max = MEMBLOCK_ALLOC_ACCESSIBLE;
> + memblock_mark_nomap(crashk_low_res.start,
> + resource_size(&crashk_low_res));
> + }
> +
> crash_size = PAGE_ALIGN(crash_size);
>
> if (crash_base == 0) {
> /* Current arm64 boot protocol requires 2MB alignment */
> - crash_base = memblock_find_in_range(0, arm64_dma32_phys_limit,
> - crash_size, SZ_2M);
> + crash_base = memblock_find_in_range(0, crash_max, crash_size,
> + SZ_2M);
> if (crash_base == 0) {
> pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
> crash_size);
> --
> 2.20.1
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
^ permalink raw reply
* Re: [PATCH v8 1/5] x86: kdump: move reserve_crashkernel_low() into crash_core.c
From: Baoquan He @ 2020-05-26 0:56 UTC (permalink / raw)
To: Chen Zhou
Cc: tglx, mingo, catalin.marinas, will, dyoung, robh+dt,
John.p.donnelly, arnd, devicetree, linux-doc, kexec, linux-kernel,
horms, guohanjun, pkushwaha, linux-arm-kernel
In-Reply-To: <20200521093805.64398-2-chenzhou10@huawei.com>
On 05/21/20 at 05:38pm, Chen Zhou wrote:
> In preparation for supporting reserve_crashkernel_low in arm64 as
> x86_64 does, move reserve_crashkernel_low() into kernel/crash_core.c.
> BTW, move x86 CRASH_ALIGN to 2M.
The reason is?
>
> Note, in arm64, we reserve low memory if and only if crashkernel=X,low
> is specified. Different with x86_64, don't set low memory automatically.
>
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Chen Zhou <chenzhou10@huawei.com>
> Tested-by: John Donnelly <John.p.donnelly@oracle.com>
> Tested-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
> ---
> arch/x86/kernel/setup.c | 66 ++++-------------------------
> include/linux/crash_core.h | 3 ++
> include/linux/kexec.h | 2 -
> kernel/crash_core.c | 85 ++++++++++++++++++++++++++++++++++++++
> kernel/kexec_core.c | 17 --------
> 5 files changed, 96 insertions(+), 77 deletions(-)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 4b3fa6cd3106..de75fec73d47 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -395,8 +395,8 @@ static void __init memblock_x86_reserve_range_setup_data(void)
>
> #ifdef CONFIG_KEXEC_CORE
>
> -/* 16M alignment for crash kernel regions */
> -#define CRASH_ALIGN SZ_16M
> +/* 2M alignment for crash kernel regions */
> +#define CRASH_ALIGN SZ_2M
>
> /*
> * Keep the crash kernel below this limit.
> @@ -419,59 +419,6 @@ static void __init memblock_x86_reserve_range_setup_data(void)
> # define CRASH_ADDR_HIGH_MAX SZ_64T
> #endif
>
> -static int __init reserve_crashkernel_low(void)
> -{
> -#ifdef CONFIG_X86_64
> - unsigned long long base, low_base = 0, low_size = 0;
> - unsigned long total_low_mem;
> - int ret;
> -
> - total_low_mem = memblock_mem_size(1UL << (32 - PAGE_SHIFT));
> -
> - /* crashkernel=Y,low */
> - ret = parse_crashkernel_low(boot_command_line, total_low_mem, &low_size, &base);
> - if (ret) {
> - /*
> - * two parts from kernel/dma/swiotlb.c:
> - * -swiotlb size: user-specified with swiotlb= or default.
> - *
> - * -swiotlb overflow buffer: now hardcoded to 32k. We round it
> - * to 8M for other buffers that may need to stay low too. Also
> - * make sure we allocate enough extra low memory so that we
> - * don't run out of DMA buffers for 32-bit devices.
> - */
> - low_size = max(swiotlb_size_or_default() + (8UL << 20), 256UL << 20);
> - } else {
> - /* passed with crashkernel=0,low ? */
> - if (!low_size)
> - return 0;
> - }
> -
> - low_base = memblock_find_in_range(0, 1ULL << 32, low_size, CRASH_ALIGN);
> - if (!low_base) {
> - pr_err("Cannot reserve %ldMB crashkernel low memory, please try smaller size.\n",
> - (unsigned long)(low_size >> 20));
> - return -ENOMEM;
> - }
> -
> - ret = memblock_reserve(low_base, low_size);
> - if (ret) {
> - pr_err("%s: Error reserving crashkernel low memblock.\n", __func__);
> - return ret;
> - }
> -
> - pr_info("Reserving %ldMB of low memory at %ldMB for crashkernel (System low RAM: %ldMB)\n",
> - (unsigned long)(low_size >> 20),
> - (unsigned long)(low_base >> 20),
> - (unsigned long)(total_low_mem >> 20));
> -
> - crashk_low_res.start = low_base;
> - crashk_low_res.end = low_base + low_size - 1;
> - insert_resource(&iomem_resource, &crashk_low_res);
> -#endif
> - return 0;
> -}
> -
> static void __init reserve_crashkernel(void)
> {
> unsigned long long crash_size, crash_base, total_mem;
> @@ -535,9 +482,12 @@ static void __init reserve_crashkernel(void)
> return;
> }
>
> - if (crash_base >= (1ULL << 32) && reserve_crashkernel_low()) {
> - memblock_free(crash_base, crash_size);
> - return;
> + if (crash_base >= (1ULL << 32)) {
> + if (reserve_crashkernel_low()) {
> + memblock_free(crash_base, crash_size);
> + return;
> + }
> + insert_resource(&iomem_resource, &crashk_low_res);
> }
>
> pr_info("Reserving %ldMB of memory at %ldMB for crashkernel (System RAM: %ldMB)\n",
> diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h
> index 525510a9f965..4df8c0bff03e 100644
> --- a/include/linux/crash_core.h
> +++ b/include/linux/crash_core.h
> @@ -63,6 +63,8 @@ phys_addr_t paddr_vmcoreinfo_note(void);
> extern unsigned char *vmcoreinfo_data;
> extern size_t vmcoreinfo_size;
> extern u32 *vmcoreinfo_note;
> +extern struct resource crashk_res;
> +extern struct resource crashk_low_res;
>
> Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type,
> void *data, size_t data_len);
> @@ -74,5 +76,6 @@ int parse_crashkernel_high(char *cmdline, unsigned long long system_ram,
> unsigned long long *crash_size, unsigned long long *crash_base);
> int parse_crashkernel_low(char *cmdline, unsigned long long system_ram,
> unsigned long long *crash_size, unsigned long long *crash_base);
> +int __init reserve_crashkernel_low(void);
>
> #endif /* LINUX_CRASH_CORE_H */
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 1776eb2e43a4..5d5d9635b18d 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -330,8 +330,6 @@ extern int kexec_load_disabled;
>
> /* Location of a reserved region to hold the crash kernel.
> */
> -extern struct resource crashk_res;
> -extern struct resource crashk_low_res;
> extern note_buf_t __percpu *crash_notes;
>
> /* flag to track if kexec reboot is in progress */
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 9f1557b98468..a7580d291c37 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -7,6 +7,8 @@
> #include <linux/crash_core.h>
> #include <linux/utsname.h>
> #include <linux/vmalloc.h>
> +#include <linux/memblock.h>
> +#include <linux/swiotlb.h>
>
> #include <asm/page.h>
> #include <asm/sections.h>
> @@ -19,6 +21,22 @@ u32 *vmcoreinfo_note;
> /* trusted vmcoreinfo, e.g. we can make a copy in the crash memory */
> static unsigned char *vmcoreinfo_data_safecopy;
>
> +/* Location of the reserved area for the crash kernel */
> +struct resource crashk_res = {
> + .name = "Crash kernel",
> + .start = 0,
> + .end = 0,
> + .flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM,
> + .desc = IORES_DESC_CRASH_KERNEL
> +};
> +struct resource crashk_low_res = {
> + .name = "Crash kernel",
> + .start = 0,
> + .end = 0,
> + .flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM,
> + .desc = IORES_DESC_CRASH_KERNEL
> +};
> +
> /*
> * parsing the "crashkernel" commandline
> *
> @@ -292,6 +310,73 @@ int __init parse_crashkernel_low(char *cmdline,
> "crashkernel=", suffix_tbl[SUFFIX_LOW]);
> }
>
> +#if defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
> +#define CRASH_ALIGN SZ_2M
> +#endif
> +
> +int __init reserve_crashkernel_low(void)
> +{
> +#if defined(CONFIG_X86_64) || defined(CONFIG_ARM64)
> + unsigned long long base, low_base = 0, low_size = 0;
> + unsigned long total_low_mem;
> + int ret;
> +
> + total_low_mem = memblock_mem_size(1UL << (32 - PAGE_SHIFT));
> +
> + /* crashkernel=Y,low */
> + ret = parse_crashkernel_low(boot_command_line, total_low_mem, &low_size,
> + &base);
> + if (ret) {
> +#ifdef CONFIG_X86_64
> + /*
> + * two parts from lib/swiotlb.c:
> + * -swiotlb size: user-specified with swiotlb= or default.
> + *
> + * -swiotlb overflow buffer: now hardcoded to 32k. We round it
> + * to 8M for other buffers that may need to stay low too. Also
> + * make sure we allocate enough extra low memory so that we
> + * don't run out of DMA buffers for 32-bit devices.
> + */
> + low_size = max(swiotlb_size_or_default() + (8UL << 20),
> + 256UL << 20);
> +#else
> + /*
> + * in arm64, reserve low memory if and only if crashkernel=X,low
> + * specified.
> + */
> + return -EINVAL;
> +#endif
> + } else {
> + /* passed with crashkernel=0,low ? */
> + if (!low_size)
> + return 0;
> + }
> +
> + low_base = memblock_find_in_range(0, 1ULL << 32, low_size, CRASH_ALIGN);
> + if (!low_base) {
> + pr_err("Cannot reserve %ldMB crashkernel low memory, please try smaller size.\n",
> + (unsigned long)(low_size >> 20));
> + return -ENOMEM;
> + }
> +
> + ret = memblock_reserve(low_base, low_size);
> + if (ret) {
> + pr_err("%s: Error reserving crashkernel low memblock.\n",
> + __func__);
> + return ret;
> + }
> +
> + pr_info("Reserving %ldMB of low memory at %ldMB for crashkernel (System low RAM: %ldMB)\n",
> + (unsigned long)(low_size >> 20),
> + (unsigned long)(low_base >> 20),
> + (unsigned long)(total_low_mem >> 20));
> +
> + crashk_low_res.start = low_base;
> + crashk_low_res.end = low_base + low_size - 1;
> +#endif
> + return 0;
> +}
> +
> Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type,
> void *data, size_t data_len)
> {
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index c19c0dad1ebe..db66bbabfff3 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -53,23 +53,6 @@ note_buf_t __percpu *crash_notes;
> /* Flag to indicate we are going to kexec a new kernel */
> bool kexec_in_progress = false;
>
> -
> -/* Location of the reserved area for the crash kernel */
> -struct resource crashk_res = {
> - .name = "Crash kernel",
> - .start = 0,
> - .end = 0,
> - .flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM,
> - .desc = IORES_DESC_CRASH_KERNEL
> -};
> -struct resource crashk_low_res = {
> - .name = "Crash kernel",
> - .start = 0,
> - .end = 0,
> - .flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM,
> - .desc = IORES_DESC_CRASH_KERNEL
> -};
> -
> int kexec_should_crash(struct task_struct *p)
> {
> /*
> --
> 2.20.1
>
>
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
^ permalink raw reply
* Re: [PATCH v4 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID DMA
From: Serge Semin @ 2020-05-25 21:36 UTC (permalink / raw)
To: Mark Brown
Cc: Serge Semin, Andy Shevchenko, Vinod Koul, Feng Tang, Grant Likely,
Georgy Vlasov, Ramil Zaripov, Alexey Malahov, Thomas Bogendoerfer,
Paul Burton, Ralf Baechle, Arnd Bergmann, Rob Herring, linux-mips,
devicetree, Wan Ahmad Zainie, Thomas Gleixner, Jarkko Nikula,
wuxu.wu, Clement Leger, Linus Walleij, linux-spi, linux-kernel
In-Reply-To: <20200525114132.GC4544@sirena.org.uk>
On Mon, May 25, 2020 at 12:41:32PM +0100, Mark Brown wrote:
> On Sat, May 23, 2020 at 11:34:10AM +0300, Serge Semin wrote:
> > On Fri, May 22, 2020 at 04:22:41PM +0100, Mark Brown wrote:
>
> > > Right, that definitely needs to be fixed then - 8MHz is indeed a totally
> > > normal clock rate for SPI so people will hit it. I guess if there's a
> > > noticable performance hit to defer to thread then we could implement
> > > both and look at how long the delay is going to be to decide which to
> > > use, that's annoyingly complicated though so if the overhead is small
> > > enough we could just not bother.
>
> > As I suggested before we can implement a solution without performance drop.
> > Just wait for the DMA completion locally in the dw_spi_dma_transfer() method and
> > return 0 instead of 1 from the transfer_one() callback. In that function we'll
> > wait while DMA finishes its business, after that we can check the Tx/Rx FIFO
> > emptiness and wait for the data to be completely transferred with delays or
> > sleeps or whatever.
>
> No extra context switches there at least, that's the main issue.
Right. There won't be extra context switch.
>
> > NOTE Currently the DW APB SSI driver doesn't set xfer->effective_speed_hz, though as
> > far as I can see that field exists there to be initialized by the SPI controller
> > driver, right? If so, strange it isn't done in any SPI drivers...
>
> Yes. Not that many people are concerned about the exact timing it turns
> out, the work that was being used for never fully made it upstream.
>
> > What do think about this?
>
> Sure.
Great. I'll send a new patchset soon. It'll fix the Tx/Rx non-empty issue in
accordance with the proposed design.
-Sergey
>
> > patchset "spi: dw: Add generic DW DMA controller support" (it's being under
> > review in this email thread) ? Anyway, if the fixup is getting to be that
> > complicated, will it have to be backported to another stable kernels?
>
> No, if it's too invasive it shouldn't be (though the stable people might
> decide they want it anyway these days :/ ).
^ permalink raw reply
* Re: [PATCH v2 2/2] hwmon: Add Baikal-T1 PVT sensor driver
From: Serge Semin @ 2020-05-25 21:26 UTC (permalink / raw)
To: Guenter Roeck
Cc: Serge Semin, Jean Delvare, Jonathan Corbet, Maxim Kaurkin,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, linux-mips, devicetree,
Jonathan Cameron, Linus Walleij, Nuno Sá, Jisheng Zhang,
Paul Cercueil, Dan Robertson, Eric Tremblay, Beniamin Bia,
Jean Delvare, Dr. David Alan Gilbert, linux-hwmon, linux-doc,
linux-kernel
In-Reply-To: <20200522141955.GA172468@roeck-us.net>
On Fri, May 22, 2020 at 07:19:55AM -0700, Guenter Roeck wrote:
> On Sun, May 10, 2020 at 01:32:11PM +0300, Serge Semin wrote:
> > Baikal-T1 SoC provides an embedded process, voltage and temperature
> > sensor to monitor an internal SoC environment (chip temperature, supply
> > voltage and process monitor) and on time detect critical situations,
> > which may cause the system instability and even damages. The IP-block
> > is based on the Analog Bits PVT sensor, but is equipped with a
> > dedicated control wrapper, which provides a MMIO registers-based access
> > to the sensor core functionality (APB3-bus based) and exposes an
> > additional functions like thresholds/data ready interrupts, its status
> > and masks, measurements timeout. All of these is used to create a hwmon
> > driver being added to the kernel by this commit.
> >
> > The driver implements support for the hardware monitoring capabilities
> > of Baikal-T1 process, voltage and temperature sensors. PVT IP-core
> > consists of one temperature and four voltage sensors, each of which is
> > implemented as a dedicated hwmon channel config.
> >
> > The driver can optionally provide the hwmon alarms for each sensor the
> > PVT controller supports. The alarms functionality is made compile-time
> > configurable due to the hardware interface implementation peculiarity,
> > which is connected with an ability to convert data from only one sensor
> > at a time. Additional limitation is that the controller performs the
> > thresholds checking synchronously with the data conversion procedure.
> > Due to these limitations in order to have the hwmon alarms
> > automatically detected the driver code must switch from one sensor to
> > another, read converted data and manually check the threshold status
> > bits. Depending on the measurements timeout settings this design may
> > cause additional burden on the system performance. By default if the
> > alarms kernel config is disabled the data conversion is performed by
> > the driver on demand when read operation is requested via corresponding
> > _input-file.
> >
> > Co-developed-by: Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> > Signed-off-by: Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> > 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: Rob Herring <robh+dt@kernel.org>
> > Cc: linux-mips@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > ---
> >
> > Changelog v2:
> > - Discard handwritten IO-access wrappers. Use normal readl/writel instead.
> > - Use generic FIELD_{GET,PREP} macros instead of handwritten ones.
> > - Since the driver depends on the OF config we can remove of_match_ptr()
> > macro utilization.
> > - Don't print error-message if no platform IRQ found. Just return an error.
> > - Remove probe-status info string printout.
> > ---
> > Documentation/hwmon/bt1-pvt.rst | 113 +++
>
> Needs to be added to index.rst.
Ok.
>
> > drivers/hwmon/Kconfig | 29 +
> > drivers/hwmon/Makefile | 1 +
> > drivers/hwmon/bt1-pvt.c | 1154 +++++++++++++++++++++++++++++++
> > drivers/hwmon/bt1-pvt.h | 237 +++++++
> > 5 files changed, 1534 insertions(+)
> > create mode 100644 Documentation/hwmon/bt1-pvt.rst
> > create mode 100644 drivers/hwmon/bt1-pvt.c
> > create mode 100644 drivers/hwmon/bt1-pvt.h
> >
> > diff --git a/Documentation/hwmon/bt1-pvt.rst b/Documentation/hwmon/bt1-pvt.rst
> > new file mode 100644
> > index 000000000000..7ad8574bce98
> > --- /dev/null
> > +++ b/Documentation/hwmon/bt1-pvt.rst
> > @@ -0,0 +1,113 @@
> > +.. SPDX-License-Identifier: GPL-2.0-only
> > +
> > +
[nip]
> > +Sysfs Attributes
> > +----------------
> > +
> > +Following is a list of all sysfs attributes that the driver provides, their
> > +permissions and a short description:
> > +
> > +=============================== ======= =======================================
> > +Name Perm Description
> > +=============================== ======= =======================================
> > +update_interval RW Measurements update interval per
> > + sensor.
> > +temp1_type RO Sensor type (always 1 as CPU embedded
> > + diode).
> > +temp1_label RO MIPS P5600 Temperature sensor.
> > +temp1_input RO Measured temperature in millidegree
> > + Celsius.
> > +temp1_min RW Low limit for temp input.
> > +temp1_max RW High limit for temp input.
> > +temp1_min_alarm RO Temperature input alarm. Returns 1 if
> > + temperature input went below min limit,
> > + 0 otherwise.
> > +temp1_max_alarm RO Temperature input alarm. Returns 1 if
> > + temperature input went above max limit,
> > + 0 otherwise.
> > +temp1_trim RW Temperature sensor trimming factor
> > + (specific to the SoC application).
>
> This needs a much better documentation as well as explanation why it needs to be
> a sysfs attribute instead of a devicetree node (ie why it needs to be
> runtime-controlled).
Trimming factor can be used to manually tune the temperature sensor characteristics
(simple speaking shifts one with respect to the base range limits) in case if in
accordance to some more reliable source of the temperature measurements, they
don't correspond to the real core temperature.
In my opinion, it would be better to have both sysfs node and DT property. The former
will be used to select a proper value at run-time when the system engineer finds
out the temperature sensor lies regarding the real core temperature. So by
picking up a proper trim value, one will make sure the sensor shows a real
temperature in accordance with a reliable reference device measurements. Then
the later (DT property) can be used to fix the determined trim value to the
particular platform configuration in the DTS file of the corresponding board.
Additionally if there is no way to alter the DT file, but a user needs to tune
the temperature sensor measurements up, then sysfs interface will be the only
way to do so.
I can also improve the description like this:
-temp1_trim RW Temperature sensor trimming factor
- (specific to the SoC application).
+temp1_trim RW Temperature sensor trimming factor
+ can be used to manually tune the
+ absolute temperature measurements up.
What do you think about new description and having both sysf node and DT
property to set the temperature trim value?
>
> > +in[0-3]_label RO MIPS P5600 Voltage sensor (either core
> > + or low/high/standard thresholds).
> > +in[0-3]_input RO Measured voltage in millivolts.
> > +in[0-3]_min RW Low limit for voltage input.
> > +in[0-3]_max RW High limit for voltage input.
> > +in[0-3]_min_alarm RO Voltage input alarm. Returns 1 if
> > + voltage input went below min limit,
> > + 0 otherwise.
> > +in[0-3]_max_alarm RO Voltage input alarm. Returns 1 if
> > + voltage input went above max limit,
> > + 0 otherwise.
> > +=============================== ======= =======================================
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 4c62f900bf7e..b80fc7d398c3 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -404,6 +404,35 @@ config SENSORS_ATXP1
> > This driver can also be built as a module. If so, the module
> > will be called atxp1.
> >
> > +config SENSORS_BT1_PVT
> > + tristate "Baikal-T1 Process, Voltage, Temperature sensor driver"
> > + depends on (MIPS_BAIKAL_T1 && OF) || COMPILE_TEST
> > + help
> > + If you say yes here you get support for Baikal-T1 PVT sensor
> > + embedded into the SoC.
> > +
> > + This driver can also be built as a module. If so, the module will be
> > + called bt1-pvt.
> > +
> > +if SENSORS_BT1_PVT
> > +
> > +config SENSORS_BT1_PVT_ALARMS
> > + bool "Enable Baikal-T1 PVT sensor alarms"
> > + default n
>
> "default n" is unnecessary. Also, instead of "if SENSORS_BT1_PVT", please
> use "depends on SENSORS_BT1_PVT".
Ok.
>
> > + help
> > + Baikal-T1 PVT IP-block provides threshold registers for each
> > + supported sensor. But the corresponding interrupts might be
> > + generated by the thresholds comparator only in synchronization with
> > + a data conversion. Additionally there is only one sensor data can
> > + be converted at a time. All of these makes the interface impossible
> > + to be used for the hwmon alarms implementation without periodic
> > + switch between the PVT sensors. By default the data conversion is
> > + performed on demand from the user-space. If this config is enabled
> > + the data conversion will be periodically performed and the data will be
> > + saved in the internal driver cache.
> > +
> > +endif
> > +
> > config SENSORS_DRIVETEMP
> > tristate "Hard disk drives with temperature sensors"
> > depends on SCSI && ATA
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index b0b9c8e57176..408b86a5e71d 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -53,6 +53,7 @@ obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
> > obj-$(CONFIG_SENSORS_ASPEED) += aspeed-pwm-tacho.o
> > obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
> > obj-$(CONFIG_SENSORS_AXI_FAN_CONTROL) += axi-fan-control.o
> > +obj-$(CONFIG_SENSORS_BT1_PVT) += bt1-pvt.o
> > obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
> > obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
> > obj-$(CONFIG_SENSORS_DA9055)+= da9055-hwmon.o
> > diff --git a/drivers/hwmon/bt1-pvt.c b/drivers/hwmon/bt1-pvt.c
> > new file mode 100644
> > index 000000000000..1a29fd01aa2b
> > --- /dev/null
> > +++ b/drivers/hwmon/bt1-pvt.c
> > @@ -0,0 +1,1154 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > + *
> > + * Authors:
> > + * Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> > + * Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > + *
> > + * Baikal-T1 Process, Voltage, Temperature sensor driver
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/bitops.h>
> > +#include <linux/mutex.h>
> > +#include <linux/seqlock.h>
> > +#include <linux/completion.h>
> > +#include <linux/limits.h>
> > +#include <linux/ktime.h>
> > +#include <linux/clk.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/hwmon-sysfs.h>
>
> Alphabetic order, please.
Ok.
>
> > +
> > +#include "bt1-pvt.h"
> > +
[nip]
> > +
> > +static irqreturn_t pvt_soft_isr(int irq, void *data)
> > +{
> > + const struct pvt_sensor_info *info;
> > + struct pvt_hwmon *pvt = data;
> > + struct pvt_cache *cache;
> > + u32 val, thres_sts, old;
> > +
> > + /*
> > + * DVALID bit will be cleared by reading the data. We need to save the
> > + * status before the next conversion happens. Threshold events will be
> > + * handled a bit later.
> > + */
> > + thres_sts = readl(pvt->regs + PVT_RAW_INTR_STAT);
> > +
> > + /*
> > + * Then lets recharge the PVT interface with the next sampling mode.
> > + * Lock the interface mutex to serialize trim, timeouts and alarm
> > + * thresholds settings.
> > + */
> > + cache = &pvt->cache[pvt->sensor];
> > + info = &pvt_info[pvt->sensor];
> > + pvt->sensor = (pvt->sensor == PVT_SENSOR_LAST) ?
> > + PVT_SENSOR_FIRST : (pvt->sensor + 1);
> > +
> > + /*
> > + * For some reason we have to mask the interrupt before changing the
> > + * mode, otherwise sometimes the temperature mode doesn't get
> > + * activated even though the actual mode in ctrl register correspond
> > + * to one. Then we read the data. By doing so we also recharge the data
> > + * conversion. After this the mode corresponding to the next
> > + * sensor in the row is set. Finally we enable the interrupts back.
> > + */
> > + mutex_lock(&pvt->iface_mtx);
> > +
> > + old = pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID,
> > + PVT_INTR_DVALID);
> > +
> > + val = readl(pvt->regs + PVT_DATA);
> > +
> > + pvt_set_mode(pvt, pvt_info[pvt->sensor].mode);
> > +
> > + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID, old);
> > +
> > + mutex_unlock(&pvt->iface_mtx);
> > +
> > + /*
> > + * We can now update the data cache with data just retrieved from the
> > + * sensor. Lock write-seqlock to make sure the reader has a coherent
> > + * data.
> > + */
> > + write_seqlock(&cache->data_seqlock);
> > +
> > + cache->data = FIELD_GET(PVT_DATA_DATA_MASK, val);
> > +
> > + write_sequnlock(&cache->data_seqlock);
> > +
> > + /*
> > + * While PVT core is doing the next mode data conversion, we'll check
> > + * whether the alarms were triggered for the current sensor. Note that
> > + * according to the documentation only one threshold IRQ status can be
> > + * set at a time, that's why if-else statement is utilized.
> > + */
> > + if ((thres_sts & info->thres_sts_lo) ^ cache->thres_sts_lo) {
> > + WRITE_ONCE(cache->thres_sts_lo, thres_sts & info->thres_sts_lo);
> > + sysfs_notify(&pvt->hwmon->kobj, NULL, info->sysfs_min_alarm);
> > + kobject_uevent(&pvt->hwmon->kobj, KOBJ_CHANGE);
> > + if (cache->thres_sts_lo) {
> > + dev_warn(pvt->dev, "%s is below the lower limit\n",
> > + info->label);
> > + } else {
> > + dev_warn(pvt->dev, "%s is back to normal\n",
> > + info->label);
> > + }
> > + } else if ((thres_sts & info->thres_sts_hi) ^ cache->thres_sts_hi) {
> > + WRITE_ONCE(cache->thres_sts_hi, thres_sts & info->thres_sts_hi);
> > + sysfs_notify(&pvt->hwmon->kobj, NULL, info->sysfs_max_alarm);
> > + kobject_uevent(&pvt->hwmon->kobj, KOBJ_CHANGE);
> > + if (cache->thres_sts_hi) {
> > + dev_warn(pvt->dev, "%s is above the upper limit\n",
> > + info->label);
> > + } else {
> > + dev_warn(pvt->dev, "%s is back to normal\n",
> > + info->label);
> > + }
> > + }
>
> The point of userspace notification is to let userspace deal with it.
> I do not think it is a good idea to clog the kernel log with this
> information. It may sound like a good idea, but in a system prone to
> exceeding temperature limits all that is left in the log will be
> repeated temperature warnings. Let userspace handle it, and let
> userspace decide if it wants to log the events somewhere.
Agreed. I'll remove the warnings.
>
> Also, please have a look at https://patchwork.kernel.org/patch/11277347/
> and check if you can use it instead of creating notifications locally.
Seems reasonable. I'll apply that patch in my tree before the patches from this
series, then use the hwmon_notify_event() to notify all the required objects at
once instead of manually pushing the notification to each of them. After a smoke
tests I'll resend the series together with your patch being initial.
Note. I will have to fix the patch a bit, since:
1) It's been created a half year ago, so there are tiny merge conflicts with
kernel 5.7-rcX.
2) hwmon_notify_event() is a bit incorrect in the incrementing channel for the
hwmon_in. It's applicable for the sattr, but not for hwmon_thermal_notify().
If I find anything else, I'll add an info about it into the patch changelog.
>
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
[nip]
> > +
> > +static int pvt_read_timeout(struct pvt_hwmon *pvt, long *val)
> > +{
> > + unsigned long rate;
> > + ktime_t kt;
> > + u32 data;
> > +
> > + rate = clk_get_rate(pvt->clks[PVT_CLOCK_REF].clk);
>
> Is this a dynamic clock ? If not, it would be better to read its frequency once
> in the probe function
No, It's not dynamic. I can do as you say, though in fact there is no much point
in it. The clock doesn't have the CLK_GET_RATE_NOCACHE flag set. Then calling
clk_get_rate() will have nearly the same performance as reading from the locally
defined reference clock frequency variable except a single lock/unlock preformed
and a few conditional statements. If I had CLK_GET_RATE_NOCACHE flag set, then
the clocks would have been dynamic and I would have had to use clk_get_rate()
here anyway. So caching the reference clock value in the PVT private data won't
change much, but will add a redundant abstraction.
So I have to ask back, are you really sure that moving the clk_get_rate() into the
probe will make the situation better? If you still think so and my reasoning didn't
change your mind, please say so explicitly.
>
> > + if (!rate)
> > + return -ENODEV;
>
> ... and return -ENODEV there if it is 0
Please, see my answer above.
>
> > +
> > + /*
> > + * Don't bother with mutex here, since we just read data from MMIO.
> > + * We also have to scale the ticks timeout up to compensate the
> > + * ms-ns-data translations.
> > + */
> > + data = readl(pvt->regs + PVT_TTIMEOUT) + 1;
> > +
> > + /*
> > + * Calculate ref-clock based delay (Ttotal) between two consecutive
> > + * data samples of the same sensor. So we first must calculate the
> > + * delay introduced by the internal ref-clock timer (Tref * Fclk).
> > + * Then add the constant timeout cuased by each conversion latency
> > + * (Tmin). The basic formulae for each conversion is following:
> > + * Ttotal = Tref * Fclk + Tmin
> > + * Note if alarms are enabled the sensors are polled one after
> > + * another, so in order to have the delay being applicable for each
> > + * sensor the requested value must be equally redistirbuted.
> > + */
> > +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> > + kt = ktime_set(PVT_SENSORS_NUM * (u64)data, 0);
> > + kt = ktime_divns(kt, rate);
> > + kt = ktime_add_ns(kt, PVT_SENSORS_NUM * PVT_TOUT_MIN);
> > +#else
> > + kt = ktime_set(data, 0);
> > + kt = ktime_divns(kt, rate);
> > + kt = ktime_add_ns(kt, PVT_TOUT_MIN);
> > +#endif
> > +
> > + /* Return the result in msec as hwmon sysfs interface requires. */
> > + *val = ktime_to_ms(kt);
>
> Personally I am not sure about the value of using ktime for those
> calculations, but that is your call.
ktime provides a very convenient and self-explanatory interface. So instead of
dividing, adding, multiplying, sec<->msec factoring, avoiding overflows, etc,
I can do this by using a well-defined ktime interface.
> AFAICS ktime_to_ms() rounds down,
> so I assume you understand that this can return 0.
Yeah. There is no problem with it here. It's just a value returned to the
user-space. We are also ok with getting zero from user-space, since in fact
there is always 1/3000 sec conversion lag. So even if user-space asks to set
zero update interval, it will be 333 usec and we don't get the IRQs flood.
>
> > +
> > + return 0;
> > +}
> > +
> > +static int pvt_write_timeout(struct pvt_hwmon *pvt, long val)
> > +{
> > + unsigned long rate;
> > + ktime_t kt;
> > + u32 data;
> > + int ret;
> > +
> > + rate = clk_get_rate(pvt->clks[PVT_CLOCK_REF].clk);
> > + if (!rate)
> > + return -ENODEV;
> > +
> > + /*
> > + * If alarms are enabled, the requested timeout must be divided
> > + * between all available sensors to have the requested delay
> > + * applicable to each individual sensor.
> > + */
> > + kt = ms_to_ktime(val);
> > +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> > + kt = ktime_divns(kt, PVT_SENSORS_NUM);
> > +#endif
> > +
> > + /*
> > + * Subtract a constant lag, which always persists due to the limited
> > + * PVT sampling rate. Make sure the timeout is non-zero.
> > + */
> > + kt = ktime_sub_ns(kt, PVT_TOUT_MIN);
> > + if (ktime_to_ns(kt) < 0)
> > + kt = ktime_set(0, 0);
>
> Doesn't that set the timeout to 0 ? That seems to conflict with the "non-zero"
> comment above. Should that be "not negative" ?
Right. It should have been "not negative". Thanks for noticing this.
>
> > +
> > + /*
> > + * Finally recalculate the timeout in terms of the reference clock
> > + * period.
> > + */
> > + data = ktime_divns(kt * rate, NSEC_PER_SEC);
> > +
> > + /*
> > + * Update the measurements delay, but lock the interface first, since
> > + * we have to disable PVT in order to have the new delay actually
> > + * updated.
> > + */
> > + ret = mutex_lock_interruptible(&pvt->iface_mtx);
> > + if (ret)
> > + return ret;
> > +
> > + pvt_set_tout(pvt, data);
> > +
> > + mutex_unlock(&pvt->iface_mtx);
> > +
> > + return 0;
> > +}
> > +
[nip]
> > +
> > +static void pvt_clear_data(void *data)
> > +{
> > + struct pvt_hwmon *pvt = data;
> > +#if !defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> > + int idx;
> > +
> > + for (idx = 0; idx < PVT_SENSORS_NUM; ++idx)
> > + complete_all(&pvt->cache[idx].conversion);
> > +#endif
> > +
> > + mutex_destroy(&pvt->iface_mtx);
> > + platform_set_drvdata(to_platform_device(pvt->dev), NULL);
>
> Unnecessary.
Ok.
>
> > +}
> > +
> > +static struct pvt_hwmon *pvt_create_data(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct pvt_hwmon *pvt;
> > + int ret, idx;
> > +
> > + pvt = devm_kzalloc(dev, sizeof(*pvt), GFP_KERNEL);
> > + if (!pvt)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + ret = devm_add_action(dev, pvt_clear_data, pvt);
> > + if (ret) {
> > + dev_err(dev, "Can't add PVT data clear action\n");
> > + return ERR_PTR(ret);
> > + }
> > +
> > + pvt->dev = dev;
> > + pvt->sensor = PVT_SENSOR_FIRST;
> > + platform_set_drvdata(pdev, pvt);
>
> I don't immediately see where this is used. Can you point to it ?
Because there is no use of it. I'll remove this setting. Thanks.
>
> > + mutex_init(&pvt->iface_mtx);
> > +
> > +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> > + for (idx = 0; idx < PVT_SENSORS_NUM; ++idx)
> > + seqlock_init(&pvt->cache[idx].data_seqlock);
> > +#else
> > + for (idx = 0; idx < PVT_SENSORS_NUM; ++idx)
> > + init_completion(&pvt->cache[idx].conversion);
> > +#endif
> > +
> > + return pvt;
> > +}
> > +
[nip]
> > +
> > +static int pvt_create_hwmon(struct pvt_hwmon *pvt)
> > +{
> > + pvt->hwmon = devm_hwmon_device_register_with_info(pvt->dev,
> > + dev_name(pvt->dev), pvt, &pvt_hwmon_info, pvt_hwmon_groups);
>
> Is the device name guaranteed to be a valid hwmon device name ?
Hm, According to the DT convention it may have dashes, which AFAICS isn't valid
hwmon device name. I'll replace dev_name() here with a string literal like
"pvt". Is it ok to have several hwmon devices with the same name?
>
> > + if (IS_ERR(pvt->hwmon)) {
> > + dev_err(pvt->dev, "Couldn't create hwmon device\n");
> > + return PTR_ERR(pvt->hwmon);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +#if defined(CONFIG_SENSORS_BT1_PVT_ALARMS)
> > +
> > +static void pvt_disable_iface(void *data)
> > +{
> > + struct pvt_hwmon *pvt = data;
> > +
> > + mutex_lock(&pvt->iface_mtx);
> > + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, 0);
> > + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID,
> > + PVT_INTR_DVALID);
> > + mutex_unlock(&pvt->iface_mtx);
> > +}
> > +
> > +static int pvt_enable_iface(struct pvt_hwmon *pvt)
> > +{
> > + int ret;
> > +
> > + ret = devm_add_action(pvt->dev, pvt_disable_iface, pvt);
> > + if (ret) {
> > + dev_err(pvt->dev, "Can't add PVT disable interface action\n");
> > + return ret;
> > + }
> > +
> > + /*
> > + * Enable sensors data conversion and IRQ. We need to lock the
> > + * interface mutex since hwmon has just been created and the
> > + * corresponding sysfs files are accessible from user-space,
> > + * which theoretically may cause races.
> > + */
>
> What if those userspace actions make a request which requires an interrupt ?
In case if CONFIG_SENSORS_BT1_PVT_ALARMS is enabled, then there is no such
action. Though user-space may set some alarm thresholds, which could cause
immediate hwmon event raise. But even if they did the event status will
be left pending and handled after we enable the IRQs in the next lines.
If CONFIG_SENSORS_BT1_PVT_ALARMS is disabled, then the IRQs are enabled/disabled
in the pvt_read_data() directly to perform the sensor conversion.
>
> > + mutex_lock(&pvt->iface_mtx);
> > + pvt_update(pvt->regs + PVT_INTR_MASK, PVT_INTR_DVALID, 0);
> > + pvt_update(pvt->regs + PVT_CTRL, PVT_CTRL_EN, PVT_CTRL_EN);
> > + mutex_unlock(&pvt->iface_mtx);
> > +
> > + return 0;
> > +}
> > +
> > +#else /* !CONFIG_SENSORS_BT1_PVT_ALARMS */
> > +
[nip]
> > diff --git a/drivers/hwmon/bt1-pvt.h b/drivers/hwmon/bt1-pvt.h
> > new file mode 100644
> > index 000000000000..7851ed947877
> > --- /dev/null
> > +++ b/drivers/hwmon/bt1-pvt.h
> > @@ -0,0 +1,237 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > + *
> > + * Baikal-T1 Process, Voltage, Temperature sensor driver
> > + */
> > +#ifndef __HWMON_BT1_PVT_H__
> > +#define __HWMON_BT1_PVT_H__
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/io.h>
> > +#include <linux/mutex.h>
> > +#include <linux/seqlock.h>
> > +#include <linux/completion.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/hwmon.h>
>
> Alphabetic order, please, and drop unnecessary includes.
Ok.
>
> > +
[nip]
> > +
> > +/*
> > + * struct pvt_sensor_info - Baikal-T1 PVT sensor informational structure
> > + * @label: hwmon sensor label.
> > + * @mode: PVT mode corresponding to the channel.
> > + * @thres_base: upper and lower threshold values of the sensor.
> > + * @thres_sts_lo: low threshold status bitfield.
> > + * @thres_sts_hi: high threshold status bitfield.
> > + * @sysfs_min_alarm: pointer to a min-alarm sysfs filename.
> > + * @sysfs_min_alarm: pointer to a max-alarm sysfs filename.
> > + */
> > +struct pvt_sensor_info {
> > + const char *label;
> > + u32 mode;
> > + unsigned long thres_base;
> > + u32 thres_sts_lo;
> > + u32 thres_sts_hi;
> > + const char *sysfs_min_alarm;
> > + const char *sysfs_max_alarm;
> > +};
> > +
> > +#define PVT_SENSOR_INFO(_label, _file, _mode, _thres) \
>
> _file seems to be a global define or variable, or at least checkpatch
> complains about its use.
Hm, mine didn't complain about it. Otherwise I would have fixed it in the first
place. Though I'll remove that name from here anyway, since the
hwmon_notify_event() notifier is utilized, which requires sensor type, attribute
and channel/index. Thus explicit sysfs_min_alarm and sysfs_max_alarm names
aren't needed anymore.
-Sergey
>
> > + { \
> > + .label = _label, \
> > + .mode = PVT_CTRL_MODE_ ##_mode, \
> > + .thres_base = PVT_ ##_thres, \
> > + .thres_sts_lo = PVT_INTR_ ##_thres## _LO, \
> > + .thres_sts_hi = PVT_INTR_ ##_thres## _HI, \
> > + .sysfs_min_alarm = _file "_min_alarm", \
> > + .sysfs_max_alarm = _file "_max_alarm" \
> > + }
> > +
[nip]
> > +
> > +#endif /* __HWMON_BT1_PVT_H__ */
^ permalink raw reply
* Re: [PATCH 3/3] hwrng: ba431-rng: add support for BA431 hwrng
From: Arnd Bergmann @ 2020-05-25 20:28 UTC (permalink / raw)
To: Olivier Sobrie
Cc: Matt Mackall, Herbert Xu, Rob Herring, Greg Kroah-Hartman,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, DTML,
linux-kernel@vger.kernel.org, Waleed Ziad, sebastien.rabou
In-Reply-To: <20200525195606.2941649-4-olivier.sobrie@silexinsight.com>
On Mon, May 25, 2020 at 10:07 PM Olivier Sobrie
<olivier.sobrie@silexinsight.com> wrote:
>
> Silex insight BA431 is an IP designed to generate random numbers that
> can be integrated in various FPGA.
> This driver adds support for it through the hwrng interface.
>
> This driver is used in Silex Insight Viper OEM boards.
>
> Signed-off-by: Olivier Sobrie <olivier.sobrie@silexinsight.com>
> Signed-off-by: Waleed Ziad <waleed94ziad@gmail.com>
The driver looks good to me.
Acked-by: Arnd Bergmann <arnd@arndb.de>
> drivers/char/hw_random/Kconfig | 10 ++
> drivers/char/hw_random/Makefile | 1 +
> drivers/char/hw_random/ba431-rng.c | 240 +++++++++++++++++++++++++++++
I wonder if we should move drivers/char/hw_random to its own top-level drivers
subsystem outside of drivers/char. It seems to be growing steadily and is larger
than a lot of other subsystems with currently 34 drivers in there.
Not your problem though.
> + /* Wait until the state changed */
> + for (i = 0; i < BA431_RESET_READ_STATUS_RETRIES; ++i) {
> + state = ba431_trng_get_state(ba431);
> + if (state >= BA431_STATE_STARTUP)
> + break;
> +
> + udelay(BA431_RESET_READ_STATUS_INTERVAL);
> + }
Looking for something to improve, I noticed that this loop can take over
a millisecond to time out, and it always runs in non-atomic context.
It may be better to use usleep_range() than udelay().
Arnd
^ permalink raw reply
* Re: [PATCH v3 1/8] dt-bindings: net: meson-dwmac: Add the amlogic,rx-delay-ns property
From: Pavel Machek @ 2020-05-25 20:17 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Martin Blumenstingl, robh+dt, linux-amlogic,
devicetree, jianxin.pan, davem, netdev, linux-kernel,
linux-arm-kernel
In-Reply-To: <20200525135728.GE752669@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 858 bytes --]
On Mon 2020-05-25 15:57:28, Andrew Lunn wrote:
> > > standardizing on rx-delay-ps and tx-delay-ps would make sense since that
> > > is the lowest resolution and the property would be correctly named with
> > > an unit in the name.
> >
> > Seems like similar patch is already being reviewed from Dan Murphy (?)
> > from TI.
>
> Dan is working on the PHY side. But there is probably code which can
> be shared.
>
> One question to consider, do we want the same properties names for MAC
> and PHY, or do we want to make them different, to avoid confusion?
We have same properties accross different hardware (compatible, reg),
so same property between MAC and PHY seems to make sense.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 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