by Mark Brown

[permalink] [raw]

Subject: Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

On Thu, Jan 14, 2021 at 11:26:56PM +0800, Muhammad Husaini Zulkifli wrote:
> Export inline function to encapsulate AON_CFG1 for controling the I/O Rail
> supplied voltage levels which communicate with Trusted Firmware.

Adding Sudeep for the SMCCC bits, not deleting any context for his
benefit.

> Signed-off-by: Muhammad Husaini Zulkifli <[email protected]>
> Acked-by: Andy Shevchenko <[email protected]>
> ---
> include/linux/firmware/intel/keembay.h | 82 ++++++++++++++++++++++++++
> 1 file changed, 82 insertions(+)
> create mode 100644 include/linux/firmware/intel/keembay.h
>
> diff --git a/include/linux/firmware/intel/keembay.h b/include/linux/firmware/intel/keembay.h
> new file mode 100644
> index 000000000000..f5a8dbfdb63b
> --- /dev/null
> +++ b/include/linux/firmware/intel/keembay.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Intel Keembay SOC Firmware API Layer
> + *
> + * Copyright (C) 2020, Intel Corporation
> + *
> + * Author: Muhammad Husaini Zulkifli <[email protected]>
> + */
> +
> +#ifndef __FIRMWARE_KEEMBAY_SMC_H__
> +#define __FIRMWARE_KEEMBAY_SMC_H__
> +
> +#include <linux/arm-smccc.h>
> +
> +/*
> + * This file defines an API function that can be called by a device driver in order to
> + * communicate with Trusted Firmware - A profile(TF-A) or Trusted Firmware - M profile (TF-M).
> + */
> +
> +#define KEEMBAY_SET_1V8_IO_RAIL 1
> +#define KEEMBAY_SET_3V3_IO_RAIL 0
> +
> +#define KEEMBAY_IOV_1_8V_uV 1800000
> +#define KEEMBAY_IOV_3_3V_uV 3300000
> +
> +#define KEEMBAY_SET_SD_VOLTAGE_ID 0xFF26
> +#define KEEMBAY_GET_SD_VOLTAGE_ID 0xFF2A
> +
> +#define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_32, \
> + ARM_SMCCC_OWNER_SIP, \
> + KEEMBAY_SET_SD_VOLTAGE_ID)
> +
> +#define ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_32, \
> + ARM_SMCCC_OWNER_SIP, \
> + KEEMBAY_GET_SD_VOLTAGE_ID)
> +
> +#define KEEMBAY_REG_NUM_CONSUMERS 2
> +
> +struct keembay_reg_supply {
> + struct regulator *consumer;
> +};
> +
> +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)
> +/*
> + * Voltage applied on the IO Rail is controlled from the Always On Register using specific
> + * bits in AON_CGF1 register. This is a secure register. Keem Bay SOC cannot exposed this
> + * register address to the outside world.
> + */
> +static inline int keembay_set_io_rail_supplied_voltage(int volt)
> +{
> + struct arm_smccc_res res;
> +
> + arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE, volt, &res);

There is a SCMI voltage domain protocol intended for just this use case
of controlling regulators managed by the firmware, why are you not using
that for these systems? See drivers/firmware/arm_scmi/voltage.c.

> +
> + return res.a0;
> +}
> +
> +static inline int keembay_get_io_rail_supplied_voltage(void)
> +{
> + struct arm_smccc_res res;
> +
> + arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE, &res);
> +
> + return res.a1;
> +}
> +#else
> +static inline int keembay_set_io_rail_supplied_voltage(int volt)
> +{
> + return -ENODEV;
> +}
> +
> +static inline int keembay_get_io_rail_supplied_voltage(void)
> +{
> + return -ENODEV;
> +}
> +#endif
> +
> +#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */
> --
> 2.17.1
>

Attachments:

(No filename) (3.41 kB)
signature.asc (499.00 B)
Download all attachments

2021-01-14 16:54:53

by Mark Brown

[permalink] [raw]

Subject: Re: [PATCH v1 7/9] dt-bindings: regulator: keembay: Add DT binding documentation

On Thu, Jan 14, 2021 at 11:26:58PM +0800, Muhammad Husaini Zulkifli wrote:
> Add DT Binding Documentation for Keem Bay SD Regulator.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

> +properties:
> + compatible:
> + const: regulator-keembay-sd

Device tree compatible strings should be in the format vendor,device.

> +required:
> + - compatible
> + - regulator-name

Why is regulator-name required here? This is a standard regulator
binding property which is covered by the generic regulator bindings and
should be optional since it is for use by the system integrator to
provide more user friendly diagnostic information about which supply
this is in the system. If you are attempting to use this for device
identification then you should be using different compatible strings for
the different regulators - even if you were using regulator-name you'd
need to document the values.

Attachments:

(No filename) (1.17 kB)
signature.asc (499.00 B)
Download all attachments

2021-01-14 17:17:39

On Mon, Jan 18, 2021 at 10:28:33AM +0000, Zulkifli, Muhammad Husaini wrote:

> >> There is a SCMI voltage domain protocol intended for just this use
> >> case of controlling regulators managed by the firmware, why are you
> >> not using that for these systems? See drivers/firmware/arm_scmi/voltage.c.

> From mmc maintainer's perspective, I should use the common modelling either using
> regulator framework or pinctrl to perform voltage operation. Not just directly invoke
> smccc call in the mmc driver. That is why I came up with this regulator driver to perform
> voltage operation.

The above is a standard way of controlling regulators via SMCCC which
already has a regulator driver, you're duplicating this functionality.

> >Indeed. Please switch to using the new voltage protocol added for this without
> >any extra code. You just need to wire up DT for this.

> May I know even if I wire up the DT, how should I call this from the mmc driver
> For set/get voltage operation? Any example?

There's one in the binding document for the driver.

Attachments:

(No filename) (1.05 kB)
signature.asc (499.00 B)
Download all attachments

2021-01-19 05:45:28

by Zulkifli, Muhammad Husaini

[permalink] [raw]

Subject: RE: [PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

Hi Cristian, Sudeep and Mark,

>-----Original Message-----
>From: Cristian Marussi <[email protected]>
>Sent: Monday, January 18, 2021 8:02 PM
>To: Zulkifli, Muhammad Husaini <[email protected]>
>Cc: Sudeep Holla <[email protected]>; Mark Brown
><[email protected]>; [email protected]; [email protected];
>[email protected]; [email protected]; Hunter, Adrian
><[email protected]>; [email protected]; linux-
>[email protected]; [email protected]; Shevchenko, Andriy
><[email protected]>; A, Rashmi <[email protected]>; Vaidya,
>Mahesh R <[email protected]>
>Subject: Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted
>Firmware Service call
>
>Hi
>
>sorry I'm a bit late on this.
>
>On Mon, Jan 18, 2021 at 10:28:33AM +0000, Zulkifli, Muhammad Husaini
>wrote:
>> Hi Sudeep and Mark,
>>
>> Thanks for the review. I replied inline.
>>
>> >-----Original Message-----
>> >From: Sudeep Holla <[email protected]>
>> >Sent: Saturday, January 16, 2021 2:58 AM
>> >To: Mark Brown <[email protected]>
>> >Cc: Zulkifli, Muhammad Husaini <[email protected]>;
>> >[email protected]; [email protected]; [email protected];
>> >[email protected]; Hunter, Adrian <[email protected]>;
>> >[email protected]; [email protected]; linux-
>> >[email protected]; Shevchenko, Andriy
>> ><[email protected]>; A, Rashmi <[email protected]>;
>> >Vaidya, Mahesh R <[email protected]>; Sudeep Holla
>> ><[email protected]>
>> >Subject: Re: [PATCH v1 5/9] firmware: keembay: Add support for
>> >Trusted Firmware Service call
>> >
>> >On Thu, Jan 14, 2021 at 04:48:11PM +0000, Mark Brown wrote:
>> >> On Thu, Jan 14, 2021 at 11:26:56PM +0800, Muhammad Husaini Zulkifli
>> >wrote:
>> >> > Export inline function to encapsulate AON_CFG1 for controling the
>> >> > I/O Rail supplied voltage levels which communicate with Trusted
>Firmware.
>> >>
>> >> Adding Sudeep for the SMCCC bits, not deleting any context for his
>> >> benefit.
>> >>
>> >
>> >Thanks Mark for cc-ing me and joining the dots. I completely forgot
>> >about that fact that this platform was using SCMI using SMC as
>> >transport. Sorry for that and it is my fault. I did review the
>> >SCMI/SMC support for this platform sometime in June/July last year
>> >and forgot the fact it is same platform when voltage/regulator
>> >support patches for SD/MMC was posted sometime later last year. I
>concentrated on SMCCC conventions and other details.
>>
>> Yes Sudeep. I redesigned the way I handle the smccc call. Previously it was
>handled directly in mmc driver.
>> After few discussion, we conclude to create an abstraction using
>> regulator framework to encapsulate this smccc call during set voltage
>operation. Using standard abstraction will make things easier for the
>maintainer.
>>
>> >
>> >[...]
>> >
>> >> > +#define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE
> \
>> >> > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,
> \
>> >> > + ARM_SMCCC_SMC_32, \
>> >> > + ARM_SMCCC_OWNER_SIP, \
>> >> > + KEEMBAY_SET_SD_VOLTAGE_ID)
>> >> > +
>> >> > +#define ARM_SMCCC_SIP_KEEMBAY_GET_SD_VOLTAGE
> \
>> >> > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,
> \
>> >> > + ARM_SMCCC_SMC_32, \
>> >> > + ARM_SMCCC_OWNER_SIP, \
>> >> > + KEEMBAY_GET_SD_VOLTAGE_ID)
>> >> > +
>> >> > +#define KEEMBAY_REG_NUM_CONSUMERS 2
>> >> > +
>> >> > +struct keembay_reg_supply {
>> >> > + struct regulator *consumer;
>> >> > +};
>> >> > +
>> >> > +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)
>> >> > +/*
>> >> > + * Voltage applied on the IO Rail is controlled from the Always
>> >> > +On Register using specific
>> >> > + * bits in AON_CGF1 register. This is a secure register. Keem
>> >> > +Bay SOC cannot exposed this
>> >> > + * register address to the outside world.
>> >> > + */
>> >> > +static inline int keembay_set_io_rail_supplied_voltage(int volt) {
>> >> > + struct arm_smccc_res res;
>> >> > +
>> >> > +
>> > arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTA
>> >GE, volt,
>> >> > +&res);
>> >>
>> >> There is a SCMI voltage domain protocol intended for just this use
>> >> case of controlling regulators managed by the firmware, why are you
>> >> not using that for these systems? See
>drivers/firmware/arm_scmi/voltage.c.
>>
>> From mmc maintainer's perspective, I should use the common modelling
>> either using regulator framework or pinctrl to perform voltage
>> operation. Not just directly invoke smccc call in the mmc driver. That
>> is why I came up with this regulator driver to perform voltage operation.
>>
>
>There is an SCMI regulator driver built on top of SCMIv3.0 Voltage Domain
>Protocol, so as long as your SCMI platform firmware support the new VD
>protocol you should be able to wire up a generic SCMI regulator in the DT
>(as described in the arm,scmi.txt bindings) and then just use the usual
>regulator framework ops with such SCMI regulator without the need to add
>a custom regulator using custom SMCCC calls).

I try to hook up the DT last night. Seems like the SCMI Protocol 17 is not implemented at ATF side.
Double check with ATF Team, currently we don't have SCMI voltage domain control in ARM Trusted Firmware yet
as of now, that is why even if I map the function to scmi, my call will be fail.

[ 2.648989] arm-scmi firmware:scmi: SCMI Notifications - Core Enabled.
[ 2.656157] arm-scmi firmware:scmi: SCMI Protocol v1.0 'INTEL:KMB' Firmware version 0x1
[ 2.664513] arm-scmi firmware:scmi: SCMI protocol 23 not implemented
[ 2.675898] arm-scmi firmware:scmi: SCMI protocol 17 not implemented

Any possibilities that for UHS patch we go with my current regulator driver implementation?

>
>Thanks
>
>Cristian
>
>> >>
>> >
>> >Indeed. Please switch to using the new voltage protocol added for this
>without
>> >any extra code. You just need to wire up DT for this.
>>
>> May I know even if I wire up the DT, how should I call this from the mmc
>driver
>> For set/get voltage operation? Any example?
>>
>> >
>> >Just for curiosity, where is SCMI platform firmware implemented ? On
>Cortex-
>> >A, secure side or external processor. Does this platform run TF-A ?
>>
>> The KMB SCMI framework is implemented in secure runtime firmware (TF-A
>BL31).
>> Hopefully I am answering your question.
>>
>> >
>> >--
>> >Regards,
>> >Sudeep

2021-01-19 17:26:47

by Sudeep Holla

[permalink] [raw]

Subject: Re: [PATCH v1 5/9] firmware: keembay: Add support for Trusted Firmware Service call

On Tue, Jan 19, 2021 at 02:38:32AM +0000, Zulkifli, Muhammad Husaini wrote:

[...]

>
> I try to hook up the DT last night. Seems like the SCMI Protocol 17 is not
> implemented at ATF side.

I had guessed that.

> Double check with ATF Team, currently we don't have SCMI voltage domain
> control in ARM Trusted Firmware yet as of now, that is why even if I map the
> function to scmi, my call will be fail.

Correct, but if you already have this custom SMCCC for voltage already
implemented in TF-A, I don't see it is a big deal to support voltage
protocol there.

>
> [ 2.648989] arm-scmi firmware:scmi: SCMI Notifications - Core Enabled.
> [ 2.656157] arm-scmi firmware:scmi: SCMI Protocol v1.0 'INTEL:KMB' Firmware version 0x1
> [ 2.664513] arm-scmi firmware:scmi: SCMI protocol 23 not implemented
> [ 2.675898] arm-scmi firmware:scmi: SCMI protocol 17 not implemented
>
> Any possibilities that for UHS patch we go with my current regulator driver
> implementation?

Sorry absolutely no. If this platform was not using SCMI, I wouldn't have
pushed back hard on this custom SMCCC. Please update TF-A to add this support.
There is no point in having custom interface just for this when everything
else is already using SCMI on this platform.

--
Regards,
Sudeep

2021-01-20 00:24:20

by Ulf Hansson

[permalink] [raw]

Subject: Re: [PATCH v1 0/9] mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC

On Thu, 14 Jan 2021 at 16:28, Muhammad Husaini Zulkifli
<[email protected]> wrote:
>
> Hi,
>
> This patch series adds Ultra High Speed(UHS-1) Bus Speed Mode Support for Keem Bay SoC SD Card.
> Summary of each patches as per below:
>
> Patch 1: Use of_device_get_match_data() helper to get the match-data.
> Patch 2: Convert to use np pointer instead of using pdev->dev.of_node.
> Patch 3: Add struct device *dev in probe func(), so that dev pointer can be widely use in probe to make code more readable.
> Patch 4: Change from dev_err to dev_err_probe() to avoid spamming logs when probe is deferred.
> Patch 5: Export function to be use by device driver to configure i/o voltage rail output which communicate with Trusted Firmware.
> Patch 6: Update phy and regulator supply for Keem Bay SoC.
> Patch 7: Add DT Binding for Keem Bay SoC SD Regulator.
> Patch 8: Add SD Regulator driver to support Keem Bay SoC. This is to model using standard regulator abstraction during voltage operation
> as for Keem Bay SoC, i/o voltage rail need to be configure by setting specific bit in the AON_CFG1 Register.
> AON_CFG1 Register is a secure register. Direct access to AON_CFG1 register will cause firewall violation in secure system.
> Patch 9: Add Ultra High Speed (UHS-1) Support for Keem Bay SOC. For Keem Bay hardware, two regulators are been used to change the I/O bus line voltage which are "vqmmc-supply" and "sdvrail-supply".
>
> All of these patches was tested with Keem Bay evaluation module board.
>
> Kindly help to review this patch set.
>
> Muhammad Husaini Zulkifli (9):
> mmc: sdhci-of-arasan: use of_device_get_match_data()
> mmc: sdhci-of-arasan: Convert to use np instead of pdev->dev.of_node
> mmc: sdhci-of-arasan: Add structure device pointer in probe function
> mmc: sdhci-of-arasan: Use dev_err_probe() to avoid spamming logs
> firmware: keembay: Add support for Trusted Firmware Service call
> dt-bindings: mmc: Update phy and regulator supply for Keem Bay SOC
> dt-bindings: regulator: keembay: Add DT binding documentation
> regulator: keembay: Add regulator for Keem Bay SoC
> mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC
>
> .../devicetree/bindings/mmc/arasan,sdhci.yaml | 7 +-
> .../bindings/regulator/keembay-regulator.yaml | 36 ++
> drivers/mmc/host/sdhci-of-arasan.c | 313 ++++++++++++++++--
> drivers/regulator/Kconfig | 10 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/keembay-sd-regulator.c | 112 +++++++
> include/linux/firmware/intel/keembay.h | 82 +++++
> 7 files changed, 532 insertions(+), 29 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/regulator/keembay-regulator.yaml
> create mode 100644 drivers/regulator/keembay-sd-regulator.c
> create mode 100644 include/linux/firmware/intel/keembay.h
>
> --
> 2.17.1
>

Applied patch 1 to patch 4. I assume you will be respinning the rest?

Thanks and kind regards
Uffe

2021-01-20 04:27:47

by Zulkifli, Muhammad Husaini

[permalink] [raw]

Subject: RE: [PATCH v1 0/9] mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC

Hi Ulf,

>-----Original Message-----
>From: Ulf Hansson <[email protected]>
>Sent: Tuesday, January 19, 2021 9:43 PM
>To: Zulkifli, Muhammad Husaini <[email protected]>
>Cc: Mark Brown <[email protected]>; Liam Girdwood
><[email protected]>; Rob Herring <[email protected]>; DTML
><[email protected]>; Hunter, Adrian <[email protected]>;
>Michal Simek <[email protected]>; [email protected]; Linux
>Kernel Mailing List <[email protected]>; Shevchenko, Andriy
><[email protected]>; A, Rashmi <[email protected]>; Vaidya,
>Mahesh R <[email protected]>
>Subject: Re: [PATCH v1 0/9] mmc: sdhci-of-arasan: Add UHS-1 support for
>Keem Bay SOC
>
>On Thu, 14 Jan 2021 at 16:28, Muhammad Husaini Zulkifli
><[email protected]> wrote:
>>
>> Hi,
>>
>> This patch series adds Ultra High Speed(UHS-1) Bus Speed Mode Support
>for Keem Bay SoC SD Card.
>> Summary of each patches as per below:
>>
>> Patch 1: Use of_device_get_match_data() helper to get the match-data.
>> Patch 2: Convert to use np pointer instead of using pdev->dev.of_node.
>> Patch 3: Add struct device *dev in probe func(), so that dev pointer can be
>widely use in probe to make code more readable.
>> Patch 4: Change from dev_err to dev_err_probe() to avoid spamming logs
>when probe is deferred.
>> Patch 5: Export function to be use by device driver to configure i/o voltage
>rail output which communicate with Trusted Firmware.
>> Patch 6: Update phy and regulator supply for Keem Bay SoC.
>> Patch 7: Add DT Binding for Keem Bay SoC SD Regulator.
>> Patch 8: Add SD Regulator driver to support Keem Bay SoC. This is to
>> model using standard regulator abstraction during voltage operation as for
>Keem Bay SoC, i/o voltage rail need to be configure by setting specific bit in
>the AON_CFG1 Register.
>> AON_CFG1 Register is a secure register. Direct access to AON_CFG1 register
>will cause firewall violation in secure system.
>> Patch 9: Add Ultra High Speed (UHS-1) Support for Keem Bay SOC. For
>Keem Bay hardware, two regulators are been used to change the I/O bus line
>voltage which are "vqmmc-supply" and "sdvrail-supply".
>>
>> All of these patches was tested with Keem Bay evaluation module board.
>>
>> Kindly help to review this patch set.
>>
>> Muhammad Husaini Zulkifli (9):
>> mmc: sdhci-of-arasan: use of_device_get_match_data()
>> mmc: sdhci-of-arasan: Convert to use np instead of pdev->dev.of_node
>> mmc: sdhci-of-arasan: Add structure device pointer in probe function
>> mmc: sdhci-of-arasan: Use dev_err_probe() to avoid spamming logs
>> firmware: keembay: Add support for Trusted Firmware Service call
>> dt-bindings: mmc: Update phy and regulator supply for Keem Bay SOC
>> dt-bindings: regulator: keembay: Add DT binding documentation
>> regulator: keembay: Add regulator for Keem Bay SoC
>> mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC
>>
>> .../devicetree/bindings/mmc/arasan,sdhci.yaml | 7 +-
>> .../bindings/regulator/keembay-regulator.yaml | 36 ++
>> drivers/mmc/host/sdhci-of-arasan.c | 313 ++++++++++++++++--
>> drivers/regulator/Kconfig | 10 +
>> drivers/regulator/Makefile | 1 +
>> drivers/regulator/keembay-sd-regulator.c | 112 +++++++
>> include/linux/firmware/intel/keembay.h | 82 +++++
>> 7 files changed, 532 insertions(+), 29 deletions(-) create mode
>> 100644
>> Documentation/devicetree/bindings/regulator/keembay-regulator.yaml
>> create mode 100644 drivers/regulator/keembay-sd-regulator.c
>> create mode 100644 include/linux/firmware/intel/keembay.h
>>
>> --
>> 2.17.1
>>
>
>Applied patch 1 to patch 4. I assume you will be respinning the rest?

Thanks!! Yeah. Seems like I need to request to ATF Team to implement SCMI voltage domain
control first in ARM trusted firmware.

>
>Thanks and kind regards
>Uffe

2021-01-26 02:21:44

by Rob Herring

[permalink] [raw]

Subject: Re: [PATCH v1 9/9] mmc: sdhci-of-arasan: Add UHS-1 support for Keem Bay SOC

On Thu, Jan 14, 2021 at 11:27:00PM +0800, Muhammad Husaini Zulkifli wrote:
> Keem Bay SOC can support dual voltage operations for GPIO SD pins to
> either 1.8V or 3.3V for bus IO line power. In order to operate the GPIOs
> line for Clk, Cmd and Data on Keem Bay hardware, it is important to
> configure the supplied voltage applied to their I/O Rail and the output
> of the I?C expander pin. Final Voltage applied on the GPIOs line are
> dependent by both supplied voltage rail and expander pin output as it is
> been set after passing through the voltage sense resistor.
>
> Keem Bay hardware is somewhat unique in the way of how IO bus line
> voltage are been controlled.
>
> Expander pin output is controlled by gpio-regulator. Voltage rail output
> is controlled by Keem Bay SD regulator. Keem Bay SD regulator encapsulated
> the Secure Monitor Calling Convention (SMCCC) to communicate with Trusted
> Firmware during set voltage operation.
>
> Signed-off-by: Muhammad Husaini Zulkifli <[email protected]>
> Acked-by: Adrian Hunter <[email protected]>
> Acked-by: Andy Shevchenko <[email protected]>
> ---
> drivers/mmc/host/sdhci-of-arasan.c | 263 +++++++++++++++++++++++++++++
> 1 file changed, 263 insertions(+)

> + u32 otap_delay, sel_clk_buffer;
> +
> + phys = of_parse_phandle(dev->of_node, "phys", 0);

Normally, you'd use the phy API here. Though not required from a DT
perspective.

> + if (!phys) {
> + dev_err(dev, "Can't get phys for sd0\n");
> + return -ENODEV;
> + }
> +
> + of_property_read_u32(phys, "intel,keembay-emmc-phy-otap-dly", &otap_delay);
> + of_property_read_u32(phys, "intel,keembay-emmc-phy-sel-clkbuf", &sel_clk_buffer);

Not doucmented?

For property names, I'd leave out the SoC name. Might want to use it in
the next chip.

Rob