* [PATCH v3 0/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC @ 2020-10-06 15:55 ` muhammad.husaini.zulkifli 0 siblings, 0 replies; 41+ messages in thread From: muhammad.husaini.zulkifli @ 2020-10-06 15:55 UTC (permalink / raw) To: adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: lakshmi.bai.raja.subramanian, wan.ahmad.zainie.wan.mohamad, muhammad.husaini.zulkifli, arnd From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> Hi. The first patch is to enable UHS-1 Support for Keem Bay EVM. The second patch is the header file to handle ATF call. These 2 patches was tested with Keem Bay evaluation module board. Kindly help to review this patch set. Thank you. Changes since v2: - Removed Document DT Bindings for Keembay Firmware. - Removed Firmware Driver to handle ATF Service call. - Add header file to handle API function for device driver to communicate with Arm Trusted Firmware. Changes since v1: - Add Document DT Bindings for Keembay Firmware. - Created Firmware Driver to handle ATF Service call - Provide API for arasan driver for sd card voltage changes Muhammad Husaini Zulkifli (2): mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC firmware: Keem Bay: Add support for Arm Trusted Firmware Service call drivers/mmc/host/sdhci-of-arasan.c | 127 ++++++++++++++++++ .../linux/firmware/intel/keembay_firmware.h | 46 +++++++ 2 files changed, 173 insertions(+) create mode 100644 include/linux/firmware/intel/keembay_firmware.h -- 2.17.1 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v3 0/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC @ 2020-10-06 15:55 ` muhammad.husaini.zulkifli 0 siblings, 0 replies; 41+ messages in thread From: muhammad.husaini.zulkifli @ 2020-10-06 15:55 UTC (permalink / raw) To: adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: lakshmi.bai.raja.subramanian, muhammad.husaini.zulkifli, arnd, wan.ahmad.zainie.wan.mohamad From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> Hi. The first patch is to enable UHS-1 Support for Keem Bay EVM. The second patch is the header file to handle ATF call. These 2 patches was tested with Keem Bay evaluation module board. Kindly help to review this patch set. Thank you. Changes since v2: - Removed Document DT Bindings for Keembay Firmware. - Removed Firmware Driver to handle ATF Service call. - Add header file to handle API function for device driver to communicate with Arm Trusted Firmware. Changes since v1: - Add Document DT Bindings for Keembay Firmware. - Created Firmware Driver to handle ATF Service call - Provide API for arasan driver for sd card voltage changes Muhammad Husaini Zulkifli (2): mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC firmware: Keem Bay: Add support for Arm Trusted Firmware Service call drivers/mmc/host/sdhci-of-arasan.c | 127 ++++++++++++++++++ .../linux/firmware/intel/keembay_firmware.h | 46 +++++++ 2 files changed, 173 insertions(+) create mode 100644 include/linux/firmware/intel/keembay_firmware.h -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC 2020-10-06 15:55 ` muhammad.husaini.zulkifli @ 2020-10-06 15:55 ` muhammad.husaini.zulkifli -1 siblings, 0 replies; 41+ messages in thread From: muhammad.husaini.zulkifli @ 2020-10-06 15:55 UTC (permalink / raw) To: adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: lakshmi.bai.raja.subramanian, wan.ahmad.zainie.wan.mohamad, muhammad.husaini.zulkifli, arnd From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> Voltage switching sequence is needed to support UHS-1 interface. There are 2 places to control the voltage. 1) By setting the AON register using firmware driver calling system-level platform management layer (SMC) to set the register. 2) By controlling the GPIO expander value to drive either 1.8V or 3.3V for power mux input. Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/host/sdhci-of-arasan.c | 127 +++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c index f186fbd016b1..e681e6f860ba 100644 --- a/drivers/mmc/host/sdhci-of-arasan.c +++ b/drivers/mmc/host/sdhci-of-arasan.c @@ -16,6 +16,7 @@ */ #include <linux/clk-provider.h> +#include <linux/gpio/consumer.h> #include <linux/mfd/syscon.h> #include <linux/module.h> #include <linux/of_device.h> @@ -23,6 +24,7 @@ #include <linux/regmap.h> #include <linux/of.h> #include <linux/firmware/xlnx-zynqmp.h> +#include <linux/firmware/intel/keembay_firmware.h> #include "cqhci.h" #include "sdhci-pltfm.h" @@ -150,6 +152,7 @@ struct sdhci_arasan_data { struct regmap *soc_ctl_base; const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; unsigned int quirks; + struct gpio_desc *uhs_gpio; /* Controller does not have CD wired and will not function normally without */ #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST BIT(0) @@ -361,6 +364,113 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc, return -EINVAL; } +static int sdhci_arasan_keembay_voltage_switch(struct mmc_host *mmc, + struct mmc_ios *ios) +{ + struct sdhci_host *host = mmc_priv(mmc); + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host); + u16 ctrl_2; + u16 clk; + int ret; + + switch (ios->signal_voltage) { + case MMC_SIGNAL_VOLTAGE_180: + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + clk &= ~SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); + + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + if (clk & SDHCI_CLOCK_CARD_EN) + return -EAGAIN; + + sdhci_writeb(host, SDHCI_POWER_ON | SDHCI_POWER_180, + SDHCI_POWER_CONTROL); + + /* + * Set VDDIO_B voltage to Low for 1.8V + * which is controlling by GPIO Expander. + */ + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 0); + + /* + * This is like final gatekeeper. Need to ensure changed voltage + * is settled before and after turn on this bit. + */ + usleep_range(1000, 1100); + + ret = keembay_sd_voltage_selection(KEEMBAY_SET_1V8_VOLT); + if (ret) + return ret; + + usleep_range(1000, 1100); + + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); + ctrl_2 |= SDHCI_CTRL_VDD_180; + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); + + /* Sleep for 5ms to stabilize 1.8V regulator */ + usleep_range(5000, 5500); + + /* 1.8V regulator output should be stable within 5 ms */ + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); + if (!(ctrl_2 & SDHCI_CTRL_VDD_180)) + return -EAGAIN; + + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + clk |= SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); + break; + case MMC_SIGNAL_VOLTAGE_330: + /* + * Set VDDIO_B voltage to High for 3.3V + * which is controlling by GPIO Expander. + */ + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 1); + + /* + * This is like final gatekeeper. Need to ensure changed voltage + * is settled before and after turn on this bit. + */ + usleep_range(1000, 1100); + + ret = keembay_sd_voltage_selection(KEEMBAY_SET_3V3_VOLT); + if (ret) + return ret; + + usleep_range(1000, 1100); + + /* Set 1.8V Signal Enable in the Host Control2 register to 0 */ + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); + ctrl_2 &= ~SDHCI_CTRL_VDD_180; + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); + + /* Sleep for 5ms to stabilize 3.3V regulator */ + usleep_range(5000, 5500); + + /* 3.3V regulator output should be stable within 5 ms */ + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); + if (ctrl_2 & SDHCI_CTRL_VDD_180) + return -EAGAIN; + + break; + default: + return -EINVAL; + } + + return 0; +} + +static int sdhci_arasan_keembay_select_drive_strength(struct mmc_card *card, + unsigned int max_dtr, int host_drv, + int card_drv, int *drv_type) +{ + if (card->host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_180) + *drv_type = MMC_SET_DRIVER_TYPE_C; + + return 0; +} + static const struct sdhci_ops sdhci_arasan_ops = { .set_clock = sdhci_arasan_set_clock, .get_max_clock = sdhci_pltfm_clk_get_max_clock, @@ -1521,6 +1631,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev) struct sdhci_pltfm_host *pltfm_host; struct sdhci_arasan_data *sdhci_arasan; struct device_node *np = pdev->dev.of_node; + struct device *dev = &pdev->dev; const struct sdhci_arasan_of_data *data; match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node); @@ -1600,6 +1711,22 @@ static int sdhci_arasan_probe(struct platform_device *pdev) host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; } + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { + struct gpio_desc *uhs; + + uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); + if (IS_ERR(uhs)) + return dev_err_probe(dev, PTR_ERR(uhs), "can't get uhs gpio\n"); + + sdhci_arasan->uhs_gpio = uhs; + + host->mmc_host_ops.start_signal_voltage_switch = + sdhci_arasan_keembay_voltage_switch; + + host->mmc_host_ops.select_drive_strength = + sdhci_arasan_keembay_select_drive_strength; + } + sdhci_arasan_update_baseclkfreq(host); ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, &pdev->dev); -- 2.17.1 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC @ 2020-10-06 15:55 ` muhammad.husaini.zulkifli 0 siblings, 0 replies; 41+ messages in thread From: muhammad.husaini.zulkifli @ 2020-10-06 15:55 UTC (permalink / raw) To: adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: lakshmi.bai.raja.subramanian, muhammad.husaini.zulkifli, arnd, wan.ahmad.zainie.wan.mohamad From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> Voltage switching sequence is needed to support UHS-1 interface. There are 2 places to control the voltage. 1) By setting the AON register using firmware driver calling system-level platform management layer (SMC) to set the register. 2) By controlling the GPIO expander value to drive either 1.8V or 3.3V for power mux input. Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/host/sdhci-of-arasan.c | 127 +++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c index f186fbd016b1..e681e6f860ba 100644 --- a/drivers/mmc/host/sdhci-of-arasan.c +++ b/drivers/mmc/host/sdhci-of-arasan.c @@ -16,6 +16,7 @@ */ #include <linux/clk-provider.h> +#include <linux/gpio/consumer.h> #include <linux/mfd/syscon.h> #include <linux/module.h> #include <linux/of_device.h> @@ -23,6 +24,7 @@ #include <linux/regmap.h> #include <linux/of.h> #include <linux/firmware/xlnx-zynqmp.h> +#include <linux/firmware/intel/keembay_firmware.h> #include "cqhci.h" #include "sdhci-pltfm.h" @@ -150,6 +152,7 @@ struct sdhci_arasan_data { struct regmap *soc_ctl_base; const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; unsigned int quirks; + struct gpio_desc *uhs_gpio; /* Controller does not have CD wired and will not function normally without */ #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST BIT(0) @@ -361,6 +364,113 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc, return -EINVAL; } +static int sdhci_arasan_keembay_voltage_switch(struct mmc_host *mmc, + struct mmc_ios *ios) +{ + struct sdhci_host *host = mmc_priv(mmc); + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host); + u16 ctrl_2; + u16 clk; + int ret; + + switch (ios->signal_voltage) { + case MMC_SIGNAL_VOLTAGE_180: + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + clk &= ~SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); + + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + if (clk & SDHCI_CLOCK_CARD_EN) + return -EAGAIN; + + sdhci_writeb(host, SDHCI_POWER_ON | SDHCI_POWER_180, + SDHCI_POWER_CONTROL); + + /* + * Set VDDIO_B voltage to Low for 1.8V + * which is controlling by GPIO Expander. + */ + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 0); + + /* + * This is like final gatekeeper. Need to ensure changed voltage + * is settled before and after turn on this bit. + */ + usleep_range(1000, 1100); + + ret = keembay_sd_voltage_selection(KEEMBAY_SET_1V8_VOLT); + if (ret) + return ret; + + usleep_range(1000, 1100); + + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); + ctrl_2 |= SDHCI_CTRL_VDD_180; + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); + + /* Sleep for 5ms to stabilize 1.8V regulator */ + usleep_range(5000, 5500); + + /* 1.8V regulator output should be stable within 5 ms */ + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); + if (!(ctrl_2 & SDHCI_CTRL_VDD_180)) + return -EAGAIN; + + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); + clk |= SDHCI_CLOCK_CARD_EN; + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); + break; + case MMC_SIGNAL_VOLTAGE_330: + /* + * Set VDDIO_B voltage to High for 3.3V + * which is controlling by GPIO Expander. + */ + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 1); + + /* + * This is like final gatekeeper. Need to ensure changed voltage + * is settled before and after turn on this bit. + */ + usleep_range(1000, 1100); + + ret = keembay_sd_voltage_selection(KEEMBAY_SET_3V3_VOLT); + if (ret) + return ret; + + usleep_range(1000, 1100); + + /* Set 1.8V Signal Enable in the Host Control2 register to 0 */ + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); + ctrl_2 &= ~SDHCI_CTRL_VDD_180; + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); + + /* Sleep for 5ms to stabilize 3.3V regulator */ + usleep_range(5000, 5500); + + /* 3.3V regulator output should be stable within 5 ms */ + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); + if (ctrl_2 & SDHCI_CTRL_VDD_180) + return -EAGAIN; + + break; + default: + return -EINVAL; + } + + return 0; +} + +static int sdhci_arasan_keembay_select_drive_strength(struct mmc_card *card, + unsigned int max_dtr, int host_drv, + int card_drv, int *drv_type) +{ + if (card->host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_180) + *drv_type = MMC_SET_DRIVER_TYPE_C; + + return 0; +} + static const struct sdhci_ops sdhci_arasan_ops = { .set_clock = sdhci_arasan_set_clock, .get_max_clock = sdhci_pltfm_clk_get_max_clock, @@ -1521,6 +1631,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev) struct sdhci_pltfm_host *pltfm_host; struct sdhci_arasan_data *sdhci_arasan; struct device_node *np = pdev->dev.of_node; + struct device *dev = &pdev->dev; const struct sdhci_arasan_of_data *data; match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node); @@ -1600,6 +1711,22 @@ static int sdhci_arasan_probe(struct platform_device *pdev) host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; } + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { + struct gpio_desc *uhs; + + uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); + if (IS_ERR(uhs)) + return dev_err_probe(dev, PTR_ERR(uhs), "can't get uhs gpio\n"); + + sdhci_arasan->uhs_gpio = uhs; + + host->mmc_host_ops.start_signal_voltage_switch = + sdhci_arasan_keembay_voltage_switch; + + host->mmc_host_ops.select_drive_strength = + sdhci_arasan_keembay_select_drive_strength; + } + sdhci_arasan_update_baseclkfreq(host); ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, &pdev->dev); -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC 2020-10-06 15:55 ` muhammad.husaini.zulkifli @ 2020-10-07 8:33 ` Michal Simek -1 siblings, 0 replies; 41+ messages in thread From: Michal Simek @ 2020-10-07 8:33 UTC (permalink / raw) To: muhammad.husaini.zulkifli, adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: lakshmi.bai.raja.subramanian, wan.ahmad.zainie.wan.mohamad, arnd On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > > Voltage switching sequence is needed to support UHS-1 interface. > There are 2 places to control the voltage. > 1) By setting the AON register using firmware driver calling > system-level platform management layer (SMC) to set the register. > 2) By controlling the GPIO expander value to drive either 1.8V or 3.3V > for power mux input. > > Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> > Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-of-arasan.c | 127 +++++++++++++++++++++++++++++ > 1 file changed, 127 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c > index f186fbd016b1..e681e6f860ba 100644 > --- a/drivers/mmc/host/sdhci-of-arasan.c > +++ b/drivers/mmc/host/sdhci-of-arasan.c > @@ -16,6 +16,7 @@ > */ > > #include <linux/clk-provider.h> > +#include <linux/gpio/consumer.h> > #include <linux/mfd/syscon.h> > #include <linux/module.h> > #include <linux/of_device.h> > @@ -23,6 +24,7 @@ > #include <linux/regmap.h> > #include <linux/of.h> > #include <linux/firmware/xlnx-zynqmp.h> > +#include <linux/firmware/intel/keembay_firmware.h> > > #include "cqhci.h" > #include "sdhci-pltfm.h" > @@ -150,6 +152,7 @@ struct sdhci_arasan_data { > struct regmap *soc_ctl_base; > const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; > unsigned int quirks; > + struct gpio_desc *uhs_gpio; > > /* Controller does not have CD wired and will not function normally without */ > #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST BIT(0) > @@ -361,6 +364,113 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc, > return -EINVAL; > } > > +static int sdhci_arasan_keembay_voltage_switch(struct mmc_host *mmc, > + struct mmc_ios *ios) > +{ > + struct sdhci_host *host = mmc_priv(mmc); > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host); > + u16 ctrl_2; > + u16 clk; nit: put it to one line. > + int ret; > + > + switch (ios->signal_voltage) { > + case MMC_SIGNAL_VOLTAGE_180: > + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); nit: double space > + clk &= ~SDHCI_CLOCK_CARD_EN; > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); > + > + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); nit: double space again. > + if (clk & SDHCI_CLOCK_CARD_EN) > + return -EAGAIN; > + > + sdhci_writeb(host, SDHCI_POWER_ON | SDHCI_POWER_180, > + SDHCI_POWER_CONTROL); > + > + /* > + * Set VDDIO_B voltage to Low for 1.8V > + * which is controlling by GPIO Expander. > + */ > + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 0); > + > + /* > + * This is like final gatekeeper. Need to ensure changed voltage > + * is settled before and after turn on this bit. > + */ > + usleep_range(1000, 1100); > + > + ret = keembay_sd_voltage_selection(KEEMBAY_SET_1V8_VOLT); > + if (ret) > + return ret; > + > + usleep_range(1000, 1100); > + > + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + ctrl_2 |= SDHCI_CTRL_VDD_180; > + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); > + > + /* Sleep for 5ms to stabilize 1.8V regulator */ > + usleep_range(5000, 5500); > + > + /* 1.8V regulator output should be stable within 5 ms */ > + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + if (!(ctrl_2 & SDHCI_CTRL_VDD_180)) > + return -EAGAIN; > + > + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > + clk |= SDHCI_CLOCK_CARD_EN; > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); > + break; > + case MMC_SIGNAL_VOLTAGE_330: > + /* > + * Set VDDIO_B voltage to High for 3.3V > + * which is controlling by GPIO Expander. > + */ > + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 1); > + > + /* > + * This is like final gatekeeper. Need to ensure changed voltage > + * is settled before and after turn on this bit. > + */ > + usleep_range(1000, 1100); > + > + ret = keembay_sd_voltage_selection(KEEMBAY_SET_3V3_VOLT); > + if (ret) > + return ret; > + > + usleep_range(1000, 1100); > + > + /* Set 1.8V Signal Enable in the Host Control2 register to 0 */ > + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + ctrl_2 &= ~SDHCI_CTRL_VDD_180; > + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); > + > + /* Sleep for 5ms to stabilize 3.3V regulator */ > + usleep_range(5000, 5500); > + > + /* 3.3V regulator output should be stable within 5 ms */ > + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + if (ctrl_2 & SDHCI_CTRL_VDD_180) > + return -EAGAIN; > + > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int sdhci_arasan_keembay_select_drive_strength(struct mmc_card *card, > + unsigned int max_dtr, int host_drv, > + int card_drv, int *drv_type) > +{ > + if (card->host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_180) > + *drv_type = MMC_SET_DRIVER_TYPE_C; > + > + return 0; > +} > + > static const struct sdhci_ops sdhci_arasan_ops = { > .set_clock = sdhci_arasan_set_clock, > .get_max_clock = sdhci_pltfm_clk_get_max_clock, > @@ -1521,6 +1631,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev) > struct sdhci_pltfm_host *pltfm_host; > struct sdhci_arasan_data *sdhci_arasan; > struct device_node *np = pdev->dev.of_node; > + struct device *dev = &pdev->dev; nit: I got this but as I see 3 lines below maybe would be better to use it everywhere but it can be done in separate patch. > const struct sdhci_arasan_of_data *data; > > match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node); > @@ -1600,6 +1711,22 @@ static int sdhci_arasan_probe(struct platform_device *pdev) > host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; > } > > + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { > + struct gpio_desc *uhs; > + > + uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); I can't see change in dt binding to record uhs gpio. Better sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); then you can avoid uhs variable. > + if (IS_ERR(uhs)) > + return dev_err_probe(dev, PTR_ERR(uhs), "can't get uhs gpio\n"); > + > + sdhci_arasan->uhs_gpio = uhs; > + > + host->mmc_host_ops.start_signal_voltage_switch = > + sdhci_arasan_keembay_voltage_switch; > + > + host->mmc_host_ops.select_drive_strength = > + sdhci_arasan_keembay_select_drive_strength; > + } > + > sdhci_arasan_update_baseclkfreq(host); > > ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, &pdev->dev); > Thanks, Michal ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC @ 2020-10-07 8:33 ` Michal Simek 0 siblings, 0 replies; 41+ messages in thread From: Michal Simek @ 2020-10-07 8:33 UTC (permalink / raw) To: muhammad.husaini.zulkifli, adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: lakshmi.bai.raja.subramanian, arnd, wan.ahmad.zainie.wan.mohamad On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > > Voltage switching sequence is needed to support UHS-1 interface. > There are 2 places to control the voltage. > 1) By setting the AON register using firmware driver calling > system-level platform management layer (SMC) to set the register. > 2) By controlling the GPIO expander value to drive either 1.8V or 3.3V > for power mux input. > > Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> > Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-of-arasan.c | 127 +++++++++++++++++++++++++++++ > 1 file changed, 127 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c > index f186fbd016b1..e681e6f860ba 100644 > --- a/drivers/mmc/host/sdhci-of-arasan.c > +++ b/drivers/mmc/host/sdhci-of-arasan.c > @@ -16,6 +16,7 @@ > */ > > #include <linux/clk-provider.h> > +#include <linux/gpio/consumer.h> > #include <linux/mfd/syscon.h> > #include <linux/module.h> > #include <linux/of_device.h> > @@ -23,6 +24,7 @@ > #include <linux/regmap.h> > #include <linux/of.h> > #include <linux/firmware/xlnx-zynqmp.h> > +#include <linux/firmware/intel/keembay_firmware.h> > > #include "cqhci.h" > #include "sdhci-pltfm.h" > @@ -150,6 +152,7 @@ struct sdhci_arasan_data { > struct regmap *soc_ctl_base; > const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; > unsigned int quirks; > + struct gpio_desc *uhs_gpio; > > /* Controller does not have CD wired and will not function normally without */ > #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST BIT(0) > @@ -361,6 +364,113 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc, > return -EINVAL; > } > > +static int sdhci_arasan_keembay_voltage_switch(struct mmc_host *mmc, > + struct mmc_ios *ios) > +{ > + struct sdhci_host *host = mmc_priv(mmc); > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host); > + u16 ctrl_2; > + u16 clk; nit: put it to one line. > + int ret; > + > + switch (ios->signal_voltage) { > + case MMC_SIGNAL_VOLTAGE_180: > + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); nit: double space > + clk &= ~SDHCI_CLOCK_CARD_EN; > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); > + > + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); nit: double space again. > + if (clk & SDHCI_CLOCK_CARD_EN) > + return -EAGAIN; > + > + sdhci_writeb(host, SDHCI_POWER_ON | SDHCI_POWER_180, > + SDHCI_POWER_CONTROL); > + > + /* > + * Set VDDIO_B voltage to Low for 1.8V > + * which is controlling by GPIO Expander. > + */ > + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 0); > + > + /* > + * This is like final gatekeeper. Need to ensure changed voltage > + * is settled before and after turn on this bit. > + */ > + usleep_range(1000, 1100); > + > + ret = keembay_sd_voltage_selection(KEEMBAY_SET_1V8_VOLT); > + if (ret) > + return ret; > + > + usleep_range(1000, 1100); > + > + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + ctrl_2 |= SDHCI_CTRL_VDD_180; > + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); > + > + /* Sleep for 5ms to stabilize 1.8V regulator */ > + usleep_range(5000, 5500); > + > + /* 1.8V regulator output should be stable within 5 ms */ > + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + if (!(ctrl_2 & SDHCI_CTRL_VDD_180)) > + return -EAGAIN; > + > + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > + clk |= SDHCI_CLOCK_CARD_EN; > + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); > + break; > + case MMC_SIGNAL_VOLTAGE_330: > + /* > + * Set VDDIO_B voltage to High for 3.3V > + * which is controlling by GPIO Expander. > + */ > + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 1); > + > + /* > + * This is like final gatekeeper. Need to ensure changed voltage > + * is settled before and after turn on this bit. > + */ > + usleep_range(1000, 1100); > + > + ret = keembay_sd_voltage_selection(KEEMBAY_SET_3V3_VOLT); > + if (ret) > + return ret; > + > + usleep_range(1000, 1100); > + > + /* Set 1.8V Signal Enable in the Host Control2 register to 0 */ > + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + ctrl_2 &= ~SDHCI_CTRL_VDD_180; > + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); > + > + /* Sleep for 5ms to stabilize 3.3V regulator */ > + usleep_range(5000, 5500); > + > + /* 3.3V regulator output should be stable within 5 ms */ > + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > + if (ctrl_2 & SDHCI_CTRL_VDD_180) > + return -EAGAIN; > + > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int sdhci_arasan_keembay_select_drive_strength(struct mmc_card *card, > + unsigned int max_dtr, int host_drv, > + int card_drv, int *drv_type) > +{ > + if (card->host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_180) > + *drv_type = MMC_SET_DRIVER_TYPE_C; > + > + return 0; > +} > + > static const struct sdhci_ops sdhci_arasan_ops = { > .set_clock = sdhci_arasan_set_clock, > .get_max_clock = sdhci_pltfm_clk_get_max_clock, > @@ -1521,6 +1631,7 @@ static int sdhci_arasan_probe(struct platform_device *pdev) > struct sdhci_pltfm_host *pltfm_host; > struct sdhci_arasan_data *sdhci_arasan; > struct device_node *np = pdev->dev.of_node; > + struct device *dev = &pdev->dev; nit: I got this but as I see 3 lines below maybe would be better to use it everywhere but it can be done in separate patch. > const struct sdhci_arasan_of_data *data; > > match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node); > @@ -1600,6 +1711,22 @@ static int sdhci_arasan_probe(struct platform_device *pdev) > host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; > } > > + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { > + struct gpio_desc *uhs; > + > + uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); I can't see change in dt binding to record uhs gpio. Better sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); then you can avoid uhs variable. > + if (IS_ERR(uhs)) > + return dev_err_probe(dev, PTR_ERR(uhs), "can't get uhs gpio\n"); > + > + sdhci_arasan->uhs_gpio = uhs; > + > + host->mmc_host_ops.start_signal_voltage_switch = > + sdhci_arasan_keembay_voltage_switch; > + > + host->mmc_host_ops.select_drive_strength = > + sdhci_arasan_keembay_select_drive_strength; > + } > + > sdhci_arasan_update_baseclkfreq(host); > > ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, &pdev->dev); > Thanks, Michal _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC 2020-10-07 8:33 ` Michal Simek @ 2020-10-07 8:55 ` Andy Shevchenko -1 siblings, 0 replies; 41+ messages in thread From: Andy Shevchenko @ 2020-10-07 8:55 UTC (permalink / raw) To: Michal Simek Cc: muhammad.husaini.zulkifli, Adrian Hunter, Sudeep Holla, Ulf Hansson, linux-mmc, linux-arm Mailing List, Linux Kernel Mailing List, lakshmi.bai.raja.subramanian, Wan Ahmad Zainie, Arnd Bergmann On Wed, Oct 7, 2020 at 11:38 AM Michal Simek <michal.simek@xilinx.com> wrote: > On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: ... > > + /* > > + * This is like final gatekeeper. Need to ensure changed voltage like a final > > + * is settled before and after turn on this bit. > > + */ ... > > + /* > > + * This is like final gatekeeper. Need to ensure changed voltage Likewise. > > + * is settled before and after turn on this bit. > > + */ ... > > + struct device *dev = &pdev->dev; > > nit: I got this but as I see 3 lines below maybe would be better to use > it everywhere but it can be done in separate patch. In that case I think it would be better to have that patch first. It make follow up code cleaner. ... > > + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { > > + struct gpio_desc *uhs; > > + > > + uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); > > I can't see change in dt binding to record uhs gpio. > > > Better > sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", > GPIOD_OUT_HIGH); > > then you can avoid uhs variable. Actually it's readability vs. additional variable. It was my suggestion to have a variable to make readability better. Are you insisting on this change? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC @ 2020-10-07 8:55 ` Andy Shevchenko 0 siblings, 0 replies; 41+ messages in thread From: Andy Shevchenko @ 2020-10-07 8:55 UTC (permalink / raw) To: Michal Simek Cc: Ulf Hansson, Arnd Bergmann, lakshmi.bai.raja.subramanian, linux-mmc, Adrian Hunter, Linux Kernel Mailing List, Wan Ahmad Zainie, Sudeep Holla, muhammad.husaini.zulkifli, linux-arm Mailing List On Wed, Oct 7, 2020 at 11:38 AM Michal Simek <michal.simek@xilinx.com> wrote: > On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: ... > > + /* > > + * This is like final gatekeeper. Need to ensure changed voltage like a final > > + * is settled before and after turn on this bit. > > + */ ... > > + /* > > + * This is like final gatekeeper. Need to ensure changed voltage Likewise. > > + * is settled before and after turn on this bit. > > + */ ... > > + struct device *dev = &pdev->dev; > > nit: I got this but as I see 3 lines below maybe would be better to use > it everywhere but it can be done in separate patch. In that case I think it would be better to have that patch first. It make follow up code cleaner. ... > > + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { > > + struct gpio_desc *uhs; > > + > > + uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); > > I can't see change in dt binding to record uhs gpio. > > > Better > sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", > GPIOD_OUT_HIGH); > > then you can avoid uhs variable. Actually it's readability vs. additional variable. It was my suggestion to have a variable to make readability better. Are you insisting on this change? -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC 2020-10-07 8:55 ` Andy Shevchenko @ 2020-10-07 9:10 ` Michal Simek -1 siblings, 0 replies; 41+ messages in thread From: Michal Simek @ 2020-10-07 9:10 UTC (permalink / raw) To: Andy Shevchenko, Michal Simek Cc: muhammad.husaini.zulkifli, Adrian Hunter, Sudeep Holla, Ulf Hansson, linux-mmc, linux-arm Mailing List, Linux Kernel Mailing List, lakshmi.bai.raja.subramanian, Wan Ahmad Zainie, Arnd Bergmann On 07. 10. 20 10:55, Andy Shevchenko wrote: > On Wed, Oct 7, 2020 at 11:38 AM Michal Simek <michal.simek@xilinx.com> wrote: >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > > ... > >>> + /* >>> + * This is like final gatekeeper. Need to ensure changed voltage > > like a final > >>> + * is settled before and after turn on this bit. >>> + */ > > ... > >>> + /* >>> + * This is like final gatekeeper. Need to ensure changed voltage > > Likewise. > >>> + * is settled before and after turn on this bit. >>> + */ > > ... > >>> + struct device *dev = &pdev->dev; >> >> nit: I got this but as I see 3 lines below maybe would be better to use >> it everywhere but it can be done in separate patch. > > In that case I think it would be better to have that patch first. It > make follow up code cleaner. > > ... > >>> + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { >>> + struct gpio_desc *uhs; >>> + >>> + uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); >> >> I can't see change in dt binding to record uhs gpio. >> >> >> Better >> sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", >> GPIOD_OUT_HIGH); >> >> then you can avoid uhs variable. > > Actually it's readability vs. additional variable. It was my > suggestion to have a variable to make readability better. > Are you insisting on this change? I understand that it is just about preference. I would use it directly not to deal with it. If your preference is via variable I am fine with it too. Thanks, Michal ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC @ 2020-10-07 9:10 ` Michal Simek 0 siblings, 0 replies; 41+ messages in thread From: Michal Simek @ 2020-10-07 9:10 UTC (permalink / raw) To: Andy Shevchenko, Michal Simek Cc: Ulf Hansson, Arnd Bergmann, lakshmi.bai.raja.subramanian, linux-mmc, Adrian Hunter, Linux Kernel Mailing List, Wan Ahmad Zainie, Sudeep Holla, muhammad.husaini.zulkifli, linux-arm Mailing List On 07. 10. 20 10:55, Andy Shevchenko wrote: > On Wed, Oct 7, 2020 at 11:38 AM Michal Simek <michal.simek@xilinx.com> wrote: >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > > ... > >>> + /* >>> + * This is like final gatekeeper. Need to ensure changed voltage > > like a final > >>> + * is settled before and after turn on this bit. >>> + */ > > ... > >>> + /* >>> + * This is like final gatekeeper. Need to ensure changed voltage > > Likewise. > >>> + * is settled before and after turn on this bit. >>> + */ > > ... > >>> + struct device *dev = &pdev->dev; >> >> nit: I got this but as I see 3 lines below maybe would be better to use >> it everywhere but it can be done in separate patch. > > In that case I think it would be better to have that patch first. It > make follow up code cleaner. > > ... > >>> + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { >>> + struct gpio_desc *uhs; >>> + >>> + uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); >> >> I can't see change in dt binding to record uhs gpio. >> >> >> Better >> sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", >> GPIOD_OUT_HIGH); >> >> then you can avoid uhs variable. > > Actually it's readability vs. additional variable. It was my > suggestion to have a variable to make readability better. > Are you insisting on this change? I understand that it is just about preference. I would use it directly not to deal with it. If your preference is via variable I am fine with it too. Thanks, Michal _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC 2020-10-07 9:10 ` Michal Simek @ 2020-10-07 12:08 ` Andy Shevchenko -1 siblings, 0 replies; 41+ messages in thread From: Andy Shevchenko @ 2020-10-07 12:08 UTC (permalink / raw) To: Michal Simek Cc: muhammad.husaini.zulkifli, Adrian Hunter, Sudeep Holla, Ulf Hansson, linux-mmc, linux-arm Mailing List, Linux Kernel Mailing List, lakshmi.bai.raja.subramanian, Wan Ahmad Zainie, Arnd Bergmann On Wed, Oct 7, 2020 at 12:10 PM Michal Simek <michal.simek@xilinx.com> wrote: > On 07. 10. 20 10:55, Andy Shevchenko wrote: > > On Wed, Oct 7, 2020 at 11:38 AM Michal Simek <michal.simek@xilinx.com> wrote: > >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: ... > >>> + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { > >>> + struct gpio_desc *uhs; > >>> + > >>> + uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); > >> > >> I can't see change in dt binding to record uhs gpio. > >> > >> > >> Better > >> sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", > >> GPIOD_OUT_HIGH); > >> > >> then you can avoid uhs variable. > > > > Actually it's readability vs. additional variable. It was my > > suggestion to have a variable to make readability better. > > Are you insisting on this change? > > I understand that it is just about preference. Correct. > I would use it directly > not to deal with it. If your preference is via variable I am fine with > it too. Yes, I suggested that and I still have the same opinion (It seems the original code tries to follow 80 character rules that's why I suggested the change). Thanks! -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC @ 2020-10-07 12:08 ` Andy Shevchenko 0 siblings, 0 replies; 41+ messages in thread From: Andy Shevchenko @ 2020-10-07 12:08 UTC (permalink / raw) To: Michal Simek Cc: Ulf Hansson, Arnd Bergmann, lakshmi.bai.raja.subramanian, linux-mmc, Adrian Hunter, Linux Kernel Mailing List, Wan Ahmad Zainie, Sudeep Holla, muhammad.husaini.zulkifli, linux-arm Mailing List On Wed, Oct 7, 2020 at 12:10 PM Michal Simek <michal.simek@xilinx.com> wrote: > On 07. 10. 20 10:55, Andy Shevchenko wrote: > > On Wed, Oct 7, 2020 at 11:38 AM Michal Simek <michal.simek@xilinx.com> wrote: > >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: ... > >>> + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { > >>> + struct gpio_desc *uhs; > >>> + > >>> + uhs = devm_gpiod_get_optional(dev, "uhs", GPIOD_OUT_HIGH); > >> > >> I can't see change in dt binding to record uhs gpio. > >> > >> > >> Better > >> sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", > >> GPIOD_OUT_HIGH); > >> > >> then you can avoid uhs variable. > > > > Actually it's readability vs. additional variable. It was my > > suggestion to have a variable to make readability better. > > Are you insisting on this change? > > I understand that it is just about preference. Correct. > I would use it directly > not to deal with it. If your preference is via variable I am fine with > it too. Yes, I suggested that and I still have the same opinion (It seems the original code tries to follow 80 character rules that's why I suggested the change). Thanks! -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC 2020-10-07 8:55 ` Andy Shevchenko @ 2020-10-07 13:28 ` Zulkifli, Muhammad Husaini -1 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 13:28 UTC (permalink / raw) To: Andy Shevchenko, Michal Simek Cc: Hunter, Adrian, Sudeep Holla, Ulf Hansson, linux-mmc, linux-arm Mailing List, Linux Kernel Mailing List, Raja Subramanian, Lakshmi Bai, Wan Mohamad, Wan Ahmad Zainie, Arnd Bergmann Hi Andy, Thanks for the feedback. I replied inline >-----Original Message----- >From: Andy Shevchenko <andy.shevchenko@gmail.com> >Sent: Wednesday, October 7, 2020 4:56 PM >To: Michal Simek <michal.simek@xilinx.com> >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >Hunter, Adrian <adrian.hunter@intel.com>; Sudeep Holla ><sudeep.holla@arm.com>; Ulf Hansson <ulf.hansson@linaro.org>; linux-mmc ><linux-mmc@vger.kernel.org>; linux-arm Mailing List <linux-arm- >kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- >kernel@vger.kernel.org>; Raja Subramanian, Lakshmi Bai ><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan Ahmad >Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>; Arnd Bergmann ><arnd@arndb.de> >Subject: Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for >Keem Bay SOC > >On Wed, Oct 7, 2020 at 11:38 AM Michal Simek <michal.simek@xilinx.com> >wrote: >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > >... > >> > + /* >> > + * This is like final gatekeeper. Need to ensure >> > + changed voltage > >like a final Noted. Done the changes > >> > + * is settled before and after turn on this bit. >> > + */ > >... > >> > + /* >> > + * This is like final gatekeeper. Need to ensure >> > + changed voltage > >Likewise. Noted. Done the changes > >> > + * is settled before and after turn on this bit. >> > + */ > >... > >> > + struct device *dev = &pdev->dev; >> >> nit: I got this but as I see 3 lines below maybe would be better to >> use it everywhere but it can be done in separate patch. > >In that case I think it would be better to have that patch first. It make follow up >code cleaner. I want to get some clarification here. Do I need a separate patch for this struct device *dev = &pdev->dev;? Can I embedded together with UHS patch? > >... > >> > + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { >> > + struct gpio_desc *uhs; >> > + >> > + uhs = devm_gpiod_get_optional(dev, "uhs", >> > + GPIOD_OUT_HIGH); >> >> I can't see change in dt binding to record uhs gpio. >> >> >> Better >> sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", >> GPIOD_OUT_HIGH); >> >> then you can avoid uhs variable. > >Actually it's readability vs. additional variable. It was my suggestion to have a >variable to make readability better. >Are you insisting on this change? > >-- >With Best Regards, >Andy Shevchenko ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC @ 2020-10-07 13:28 ` Zulkifli, Muhammad Husaini 0 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 13:28 UTC (permalink / raw) To: Andy Shevchenko, Michal Simek Cc: Ulf Hansson, Arnd Bergmann, Raja Subramanian, Lakshmi Bai, linux-mmc, Linux Kernel Mailing List, Hunter, Adrian, Sudeep Holla, Wan Mohamad, Wan Ahmad Zainie, linux-arm Mailing List Hi Andy, Thanks for the feedback. I replied inline >-----Original Message----- >From: Andy Shevchenko <andy.shevchenko@gmail.com> >Sent: Wednesday, October 7, 2020 4:56 PM >To: Michal Simek <michal.simek@xilinx.com> >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >Hunter, Adrian <adrian.hunter@intel.com>; Sudeep Holla ><sudeep.holla@arm.com>; Ulf Hansson <ulf.hansson@linaro.org>; linux-mmc ><linux-mmc@vger.kernel.org>; linux-arm Mailing List <linux-arm- >kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- >kernel@vger.kernel.org>; Raja Subramanian, Lakshmi Bai ><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan Ahmad >Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>; Arnd Bergmann ><arnd@arndb.de> >Subject: Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for >Keem Bay SOC > >On Wed, Oct 7, 2020 at 11:38 AM Michal Simek <michal.simek@xilinx.com> >wrote: >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > >... > >> > + /* >> > + * This is like final gatekeeper. Need to ensure >> > + changed voltage > >like a final Noted. Done the changes > >> > + * is settled before and after turn on this bit. >> > + */ > >... > >> > + /* >> > + * This is like final gatekeeper. Need to ensure >> > + changed voltage > >Likewise. Noted. Done the changes > >> > + * is settled before and after turn on this bit. >> > + */ > >... > >> > + struct device *dev = &pdev->dev; >> >> nit: I got this but as I see 3 lines below maybe would be better to >> use it everywhere but it can be done in separate patch. > >In that case I think it would be better to have that patch first. It make follow up >code cleaner. I want to get some clarification here. Do I need a separate patch for this struct device *dev = &pdev->dev;? Can I embedded together with UHS patch? > >... > >> > + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { >> > + struct gpio_desc *uhs; >> > + >> > + uhs = devm_gpiod_get_optional(dev, "uhs", >> > + GPIOD_OUT_HIGH); >> >> I can't see change in dt binding to record uhs gpio. >> >> >> Better >> sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", >> GPIOD_OUT_HIGH); >> >> then you can avoid uhs variable. > >Actually it's readability vs. additional variable. It was my suggestion to have a >variable to make readability better. >Are you insisting on this change? > >-- >With Best Regards, >Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC 2020-10-07 13:28 ` Zulkifli, Muhammad Husaini @ 2020-10-07 14:54 ` Andy Shevchenko -1 siblings, 0 replies; 41+ messages in thread From: Andy Shevchenko @ 2020-10-07 14:54 UTC (permalink / raw) To: Zulkifli, Muhammad Husaini Cc: Michal Simek, Hunter, Adrian, Sudeep Holla, Ulf Hansson, linux-mmc, linux-arm Mailing List, Linux Kernel Mailing List, Raja Subramanian, Lakshmi Bai, Wan Mohamad, Wan Ahmad Zainie, Arnd Bergmann On Wed, Oct 7, 2020 at 4:28 PM Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> wrote: > >From: Andy Shevchenko <andy.shevchenko@gmail.com> > >Sent: Wednesday, October 7, 2020 4:56 PM > >On Wed, Oct 7, 2020 at 11:38 AM Michal Simek <michal.simek@xilinx.com> > >wrote: > >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: ... > >> > + struct device *dev = &pdev->dev; > >> > >> nit: I got this but as I see 3 lines below maybe would be better to > >> use it everywhere but it can be done in separate patch. > > > >In that case I think it would be better to have that patch first. It make follow up > >code cleaner. > I want to get some clarification here. > Do I need a separate patch for this struct device *dev = &pdev->dev;? It should be a separate patch and better your series starts with it, so it won't interfere with new code. > Can I embedded together with UHS patch? Better to avoid merging orthogonal things together in one change. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC @ 2020-10-07 14:54 ` Andy Shevchenko 0 siblings, 0 replies; 41+ messages in thread From: Andy Shevchenko @ 2020-10-07 14:54 UTC (permalink / raw) To: Zulkifli, Muhammad Husaini Cc: Ulf Hansson, Arnd Bergmann, Linux Kernel Mailing List, linux-mmc, Michal Simek, Raja Subramanian, Lakshmi Bai, Hunter, Adrian, Sudeep Holla, Wan Mohamad, Wan Ahmad Zainie, linux-arm Mailing List On Wed, Oct 7, 2020 at 4:28 PM Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> wrote: > >From: Andy Shevchenko <andy.shevchenko@gmail.com> > >Sent: Wednesday, October 7, 2020 4:56 PM > >On Wed, Oct 7, 2020 at 11:38 AM Michal Simek <michal.simek@xilinx.com> > >wrote: > >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: ... > >> > + struct device *dev = &pdev->dev; > >> > >> nit: I got this but as I see 3 lines below maybe would be better to > >> use it everywhere but it can be done in separate patch. > > > >In that case I think it would be better to have that patch first. It make follow up > >code cleaner. > I want to get some clarification here. > Do I need a separate patch for this struct device *dev = &pdev->dev;? It should be a separate patch and better your series starts with it, so it won't interfere with new code. > Can I embedded together with UHS patch? Better to avoid merging orthogonal things together in one change. -- With Best Regards, Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC 2020-10-07 14:54 ` Andy Shevchenko @ 2020-10-07 16:08 ` Zulkifli, Muhammad Husaini -1 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 16:08 UTC (permalink / raw) To: Andy Shevchenko Cc: Michal Simek, Hunter, Adrian, Sudeep Holla, Ulf Hansson, linux-mmc, linux-arm Mailing List, Linux Kernel Mailing List, Raja Subramanian, Lakshmi Bai, Wan Mohamad, Wan Ahmad Zainie, Arnd Bergmann Hi , >-----Original Message----- >From: Andy Shevchenko <andy.shevchenko@gmail.com> >Sent: Wednesday, October 7, 2020 10:55 PM >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> >Cc: Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian ><adrian.hunter@intel.com>; Sudeep Holla <sudeep.holla@arm.com>; Ulf >Hansson <ulf.hansson@linaro.org>; linux-mmc <linux-mmc@vger.kernel.org>; >linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>; Linux Kernel >Mailing List <linux-kernel@vger.kernel.org>; Raja Subramanian, Lakshmi Bai ><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; Arnd Bergmann ><arnd@arndb.de> >Subject: Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for >Keem Bay SOC > >On Wed, Oct 7, 2020 at 4:28 PM Zulkifli, Muhammad Husaini ><muhammad.husaini.zulkifli@intel.com> wrote: >> >From: Andy Shevchenko <andy.shevchenko@gmail.com> >> >Sent: Wednesday, October 7, 2020 4:56 PM On Wed, Oct 7, 2020 at 11:38 >> >AM Michal Simek <michal.simek@xilinx.com> >> >wrote: >> >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > >... > >> >> > + struct device *dev = &pdev->dev; >> >> >> >> nit: I got this but as I see 3 lines below maybe would be better to >> >> use it everywhere but it can be done in separate patch. >> > >> >In that case I think it would be better to have that patch first. It >> >make follow up code cleaner. >> I want to get some clarification here. > >> Do I need a separate patch for this struct device *dev = &pdev->dev;? > >It should be a separate patch and better your series starts with it, so it won't >interfere with new code. > >> Can I embedded together with UHS patch? > >Better to avoid merging orthogonal things together in one change. Noted. Thanks 😉 > >-- >With Best Regards, >Andy Shevchenko ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC @ 2020-10-07 16:08 ` Zulkifli, Muhammad Husaini 0 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 16:08 UTC (permalink / raw) To: Andy Shevchenko Cc: Ulf Hansson, Arnd Bergmann, Linux Kernel Mailing List, linux-mmc, Michal Simek, Raja Subramanian, Lakshmi Bai, Hunter, Adrian, Sudeep Holla, Wan Mohamad, Wan Ahmad Zainie, linux-arm Mailing List Hi , >-----Original Message----- >From: Andy Shevchenko <andy.shevchenko@gmail.com> >Sent: Wednesday, October 7, 2020 10:55 PM >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> >Cc: Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian ><adrian.hunter@intel.com>; Sudeep Holla <sudeep.holla@arm.com>; Ulf >Hansson <ulf.hansson@linaro.org>; linux-mmc <linux-mmc@vger.kernel.org>; >linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>; Linux Kernel >Mailing List <linux-kernel@vger.kernel.org>; Raja Subramanian, Lakshmi Bai ><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; Arnd Bergmann ><arnd@arndb.de> >Subject: Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for >Keem Bay SOC > >On Wed, Oct 7, 2020 at 4:28 PM Zulkifli, Muhammad Husaini ><muhammad.husaini.zulkifli@intel.com> wrote: >> >From: Andy Shevchenko <andy.shevchenko@gmail.com> >> >Sent: Wednesday, October 7, 2020 4:56 PM On Wed, Oct 7, 2020 at 11:38 >> >AM Michal Simek <michal.simek@xilinx.com> >> >wrote: >> >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > >... > >> >> > + struct device *dev = &pdev->dev; >> >> >> >> nit: I got this but as I see 3 lines below maybe would be better to >> >> use it everywhere but it can be done in separate patch. >> > >> >In that case I think it would be better to have that patch first. It >> >make follow up code cleaner. >> I want to get some clarification here. > >> Do I need a separate patch for this struct device *dev = &pdev->dev;? > >It should be a separate patch and better your series starts with it, so it won't >interfere with new code. > >> Can I embedded together with UHS patch? > >Better to avoid merging orthogonal things together in one change. Noted. Thanks 😉 > >-- >With Best Regards, >Andy Shevchenko _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC 2020-10-07 8:33 ` Michal Simek @ 2020-10-07 13:25 ` Zulkifli, Muhammad Husaini -1 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 13:25 UTC (permalink / raw) To: Michal Simek, Hunter, Adrian, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, Wan Mohamad, Wan Ahmad Zainie, arnd Hi Michal, Thanks again for the feedback. I replied inline >-----Original Message----- >From: Michal Simek <michal.simek@xilinx.com> >Sent: Wednesday, October 7, 2020 4:34 PM >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >sudeep.holla@arm.com; ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; >linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org >Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >Wan Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >Subject: Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for >Keem Bay SOC > > > >On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> >> >> Voltage switching sequence is needed to support UHS-1 interface. >> There are 2 places to control the voltage. >> 1) By setting the AON register using firmware driver calling >> system-level platform management layer (SMC) to set the register. >> 2) By controlling the GPIO expander value to drive either 1.8V or 3.3V >> for power mux input. >> >> Signed-off-by: Muhammad Husaini Zulkifli >> <muhammad.husaini.zulkifli@intel.com> >> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> >> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> >> --- >> drivers/mmc/host/sdhci-of-arasan.c | 127 >> +++++++++++++++++++++++++++++ >> 1 file changed, 127 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-of-arasan.c >> b/drivers/mmc/host/sdhci-of-arasan.c >> index f186fbd016b1..e681e6f860ba 100644 >> --- a/drivers/mmc/host/sdhci-of-arasan.c >> +++ b/drivers/mmc/host/sdhci-of-arasan.c >> @@ -16,6 +16,7 @@ >> */ >> >> #include <linux/clk-provider.h> >> +#include <linux/gpio/consumer.h> >> #include <linux/mfd/syscon.h> >> #include <linux/module.h> >> #include <linux/of_device.h> >> @@ -23,6 +24,7 @@ >> #include <linux/regmap.h> >> #include <linux/of.h> >> #include <linux/firmware/xlnx-zynqmp.h> >> +#include <linux/firmware/intel/keembay_firmware.h> >> >> #include "cqhci.h" >> #include "sdhci-pltfm.h" >> @@ -150,6 +152,7 @@ struct sdhci_arasan_data { >> struct regmap *soc_ctl_base; >> const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; >> unsigned int quirks; >> + struct gpio_desc *uhs_gpio; >> >> /* Controller does not have CD wired and will not function normally without >*/ >> #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST BIT(0) >> @@ -361,6 +364,113 @@ static int sdhci_arasan_voltage_switch(struct >mmc_host *mmc, >> return -EINVAL; >> } >> >> +static int sdhci_arasan_keembay_voltage_switch(struct mmc_host *mmc, >> + struct mmc_ios *ios) >> +{ >> + struct sdhci_host *host = mmc_priv(mmc); >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> + struct sdhci_arasan_data *sdhci_arasan = >sdhci_pltfm_priv(pltfm_host); >> + u16 ctrl_2; >> + u16 clk; > >nit: put it to one line. Noted. Done the changes. > >> + int ret; >> + >> + switch (ios->signal_voltage) { >> + case MMC_SIGNAL_VOLTAGE_180: >> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > >nit: double space Noted. Done the changes. > >> + clk &= ~SDHCI_CLOCK_CARD_EN; >> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); >> + >> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > >nit: double space again. Noted. Done the changes. > >> + if (clk & SDHCI_CLOCK_CARD_EN) >> + return -EAGAIN; >> + >> + sdhci_writeb(host, SDHCI_POWER_ON | SDHCI_POWER_180, >> + SDHCI_POWER_CONTROL); >> + >> + /* >> + * Set VDDIO_B voltage to Low for 1.8V >> + * which is controlling by GPIO Expander. >> + */ >> + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 0); >> + >> + /* >> + * This is like final gatekeeper. Need to ensure changed voltage >> + * is settled before and after turn on this bit. >> + */ >> + usleep_range(1000, 1100); >> + >> + ret = >keembay_sd_voltage_selection(KEEMBAY_SET_1V8_VOLT); >> + if (ret) >> + return ret; >> + >> + usleep_range(1000, 1100); >> + >> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> + ctrl_2 |= SDHCI_CTRL_VDD_180; >> + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); >> + >> + /* Sleep for 5ms to stabilize 1.8V regulator */ >> + usleep_range(5000, 5500); >> + >> + /* 1.8V regulator output should be stable within 5 ms */ >> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> + if (!(ctrl_2 & SDHCI_CTRL_VDD_180)) >> + return -EAGAIN; >> + >> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); >> + clk |= SDHCI_CLOCK_CARD_EN; >> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); >> + break; >> + case MMC_SIGNAL_VOLTAGE_330: >> + /* >> + * Set VDDIO_B voltage to High for 3.3V >> + * which is controlling by GPIO Expander. >> + */ >> + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 1); >> + >> + /* >> + * This is like final gatekeeper. Need to ensure changed voltage >> + * is settled before and after turn on this bit. >> + */ >> + usleep_range(1000, 1100); >> + >> + ret = >keembay_sd_voltage_selection(KEEMBAY_SET_3V3_VOLT); >> + if (ret) >> + return ret; >> + >> + usleep_range(1000, 1100); >> + >> + /* Set 1.8V Signal Enable in the Host Control2 register to 0 */ >> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> + ctrl_2 &= ~SDHCI_CTRL_VDD_180; >> + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); >> + >> + /* Sleep for 5ms to stabilize 3.3V regulator */ >> + usleep_range(5000, 5500); >> + >> + /* 3.3V regulator output should be stable within 5 ms */ >> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> + if (ctrl_2 & SDHCI_CTRL_VDD_180) >> + return -EAGAIN; >> + >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +static int sdhci_arasan_keembay_select_drive_strength(struct mmc_card >*card, >> + unsigned int max_dtr, int host_drv, >> + int card_drv, int *drv_type) >> +{ >> + if (card->host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_180) >> + *drv_type = MMC_SET_DRIVER_TYPE_C; >> + >> + return 0; >> +} >> + >> static const struct sdhci_ops sdhci_arasan_ops = { >> .set_clock = sdhci_arasan_set_clock, >> .get_max_clock = sdhci_pltfm_clk_get_max_clock, @@ -1521,6 +1631,7 >> @@ static int sdhci_arasan_probe(struct platform_device *pdev) >> struct sdhci_pltfm_host *pltfm_host; >> struct sdhci_arasan_data *sdhci_arasan; >> struct device_node *np = pdev->dev.of_node; >> + struct device *dev = &pdev->dev; > >nit: I got this but as I see 3 lines below maybe would be better to use it >everywhere but it can be done in separate patch. > >> const struct sdhci_arasan_of_data *data; >> >> match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node); >@@ >> -1600,6 +1711,22 @@ static int sdhci_arasan_probe(struct platform_device >*pdev) >> host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; >> } >> >> + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { >> + struct gpio_desc *uhs; >> + >> + uhs = devm_gpiod_get_optional(dev, "uhs", >GPIOD_OUT_HIGH); > >I can't see change in dt binding to record uhs gpio. Noted. Done the changes. Will add binding for this > > >Better >sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", >GPIOD_OUT_HIGH); > >then you can avoid uhs variable. Will remain as it is to make it more readable. > >> + if (IS_ERR(uhs)) >> + return dev_err_probe(dev, PTR_ERR(uhs), "can't get >uhs gpio\n"); >> + >> + sdhci_arasan->uhs_gpio = uhs; >> + >> + host->mmc_host_ops.start_signal_voltage_switch = >> + sdhci_arasan_keembay_voltage_switch; >> + >> + host->mmc_host_ops.select_drive_strength = >> + sdhci_arasan_keembay_select_drive_strength; >> + } >> + >> sdhci_arasan_update_baseclkfreq(host); >> >> ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, >> &pdev->dev); >> > >Thanks, >Michal ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC @ 2020-10-07 13:25 ` Zulkifli, Muhammad Husaini 0 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 13:25 UTC (permalink / raw) To: Michal Simek, Hunter, Adrian, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, arnd, Wan Mohamad, Wan Ahmad Zainie Hi Michal, Thanks again for the feedback. I replied inline >-----Original Message----- >From: Michal Simek <michal.simek@xilinx.com> >Sent: Wednesday, October 7, 2020 4:34 PM >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >sudeep.holla@arm.com; ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; >linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org >Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >Wan Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >Subject: Re: [PATCH v3 1/2] mmc: sdhci-of-arasan: Enable UHS-1 support for >Keem Bay SOC > > > >On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> >> >> Voltage switching sequence is needed to support UHS-1 interface. >> There are 2 places to control the voltage. >> 1) By setting the AON register using firmware driver calling >> system-level platform management layer (SMC) to set the register. >> 2) By controlling the GPIO expander value to drive either 1.8V or 3.3V >> for power mux input. >> >> Signed-off-by: Muhammad Husaini Zulkifli >> <muhammad.husaini.zulkifli@intel.com> >> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com> >> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> >> --- >> drivers/mmc/host/sdhci-of-arasan.c | 127 >> +++++++++++++++++++++++++++++ >> 1 file changed, 127 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-of-arasan.c >> b/drivers/mmc/host/sdhci-of-arasan.c >> index f186fbd016b1..e681e6f860ba 100644 >> --- a/drivers/mmc/host/sdhci-of-arasan.c >> +++ b/drivers/mmc/host/sdhci-of-arasan.c >> @@ -16,6 +16,7 @@ >> */ >> >> #include <linux/clk-provider.h> >> +#include <linux/gpio/consumer.h> >> #include <linux/mfd/syscon.h> >> #include <linux/module.h> >> #include <linux/of_device.h> >> @@ -23,6 +24,7 @@ >> #include <linux/regmap.h> >> #include <linux/of.h> >> #include <linux/firmware/xlnx-zynqmp.h> >> +#include <linux/firmware/intel/keembay_firmware.h> >> >> #include "cqhci.h" >> #include "sdhci-pltfm.h" >> @@ -150,6 +152,7 @@ struct sdhci_arasan_data { >> struct regmap *soc_ctl_base; >> const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; >> unsigned int quirks; >> + struct gpio_desc *uhs_gpio; >> >> /* Controller does not have CD wired and will not function normally without >*/ >> #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST BIT(0) >> @@ -361,6 +364,113 @@ static int sdhci_arasan_voltage_switch(struct >mmc_host *mmc, >> return -EINVAL; >> } >> >> +static int sdhci_arasan_keembay_voltage_switch(struct mmc_host *mmc, >> + struct mmc_ios *ios) >> +{ >> + struct sdhci_host *host = mmc_priv(mmc); >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> + struct sdhci_arasan_data *sdhci_arasan = >sdhci_pltfm_priv(pltfm_host); >> + u16 ctrl_2; >> + u16 clk; > >nit: put it to one line. Noted. Done the changes. > >> + int ret; >> + >> + switch (ios->signal_voltage) { >> + case MMC_SIGNAL_VOLTAGE_180: >> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > >nit: double space Noted. Done the changes. > >> + clk &= ~SDHCI_CLOCK_CARD_EN; >> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); >> + >> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > >nit: double space again. Noted. Done the changes. > >> + if (clk & SDHCI_CLOCK_CARD_EN) >> + return -EAGAIN; >> + >> + sdhci_writeb(host, SDHCI_POWER_ON | SDHCI_POWER_180, >> + SDHCI_POWER_CONTROL); >> + >> + /* >> + * Set VDDIO_B voltage to Low for 1.8V >> + * which is controlling by GPIO Expander. >> + */ >> + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 0); >> + >> + /* >> + * This is like final gatekeeper. Need to ensure changed voltage >> + * is settled before and after turn on this bit. >> + */ >> + usleep_range(1000, 1100); >> + >> + ret = >keembay_sd_voltage_selection(KEEMBAY_SET_1V8_VOLT); >> + if (ret) >> + return ret; >> + >> + usleep_range(1000, 1100); >> + >> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> + ctrl_2 |= SDHCI_CTRL_VDD_180; >> + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); >> + >> + /* Sleep for 5ms to stabilize 1.8V regulator */ >> + usleep_range(5000, 5500); >> + >> + /* 1.8V regulator output should be stable within 5 ms */ >> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> + if (!(ctrl_2 & SDHCI_CTRL_VDD_180)) >> + return -EAGAIN; >> + >> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); >> + clk |= SDHCI_CLOCK_CARD_EN; >> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); >> + break; >> + case MMC_SIGNAL_VOLTAGE_330: >> + /* >> + * Set VDDIO_B voltage to High for 3.3V >> + * which is controlling by GPIO Expander. >> + */ >> + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 1); >> + >> + /* >> + * This is like final gatekeeper. Need to ensure changed voltage >> + * is settled before and after turn on this bit. >> + */ >> + usleep_range(1000, 1100); >> + >> + ret = >keembay_sd_voltage_selection(KEEMBAY_SET_3V3_VOLT); >> + if (ret) >> + return ret; >> + >> + usleep_range(1000, 1100); >> + >> + /* Set 1.8V Signal Enable in the Host Control2 register to 0 */ >> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> + ctrl_2 &= ~SDHCI_CTRL_VDD_180; >> + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); >> + >> + /* Sleep for 5ms to stabilize 3.3V regulator */ >> + usleep_range(5000, 5500); >> + >> + /* 3.3V regulator output should be stable within 5 ms */ >> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); >> + if (ctrl_2 & SDHCI_CTRL_VDD_180) >> + return -EAGAIN; >> + >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +static int sdhci_arasan_keembay_select_drive_strength(struct mmc_card >*card, >> + unsigned int max_dtr, int host_drv, >> + int card_drv, int *drv_type) >> +{ >> + if (card->host->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_180) >> + *drv_type = MMC_SET_DRIVER_TYPE_C; >> + >> + return 0; >> +} >> + >> static const struct sdhci_ops sdhci_arasan_ops = { >> .set_clock = sdhci_arasan_set_clock, >> .get_max_clock = sdhci_pltfm_clk_get_max_clock, @@ -1521,6 +1631,7 >> @@ static int sdhci_arasan_probe(struct platform_device *pdev) >> struct sdhci_pltfm_host *pltfm_host; >> struct sdhci_arasan_data *sdhci_arasan; >> struct device_node *np = pdev->dev.of_node; >> + struct device *dev = &pdev->dev; > >nit: I got this but as I see 3 lines below maybe would be better to use it >everywhere but it can be done in separate patch. > >> const struct sdhci_arasan_of_data *data; >> >> match = of_match_node(sdhci_arasan_of_match, pdev->dev.of_node); >@@ >> -1600,6 +1711,22 @@ static int sdhci_arasan_probe(struct platform_device >*pdev) >> host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; >> } >> >> + if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { >> + struct gpio_desc *uhs; >> + >> + uhs = devm_gpiod_get_optional(dev, "uhs", >GPIOD_OUT_HIGH); > >I can't see change in dt binding to record uhs gpio. Noted. Done the changes. Will add binding for this > > >Better >sdhci_arasan->uhs_gpio = devm_gpiod_get_optional(dev, "uhs", >GPIOD_OUT_HIGH); > >then you can avoid uhs variable. Will remain as it is to make it more readable. > >> + if (IS_ERR(uhs)) >> + return dev_err_probe(dev, PTR_ERR(uhs), "can't get >uhs gpio\n"); >> + >> + sdhci_arasan->uhs_gpio = uhs; >> + >> + host->mmc_host_ops.start_signal_voltage_switch = >> + sdhci_arasan_keembay_voltage_switch; >> + >> + host->mmc_host_ops.select_drive_strength = >> + sdhci_arasan_keembay_select_drive_strength; >> + } >> + >> sdhci_arasan_update_baseclkfreq(host); >> >> ret = sdhci_arasan_register_sdclk(sdhci_arasan, clk_xin, >> &pdev->dev); >> > >Thanks, >Michal _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call 2020-10-06 15:55 ` muhammad.husaini.zulkifli @ 2020-10-06 15:55 ` muhammad.husaini.zulkifli -1 siblings, 0 replies; 41+ messages in thread From: muhammad.husaini.zulkifli @ 2020-10-06 15:55 UTC (permalink / raw) To: adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: lakshmi.bai.raja.subramanian, wan.ahmad.zainie.wan.mohamad, muhammad.husaini.zulkifli, arnd From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> Add header file to handle API function for device driver to communicate with Arm Trusted Firmware. Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> --- .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 include/linux/firmware/intel/keembay_firmware.h diff --git a/include/linux/firmware/intel/keembay_firmware.h b/include/linux/firmware/intel/keembay_firmware.h new file mode 100644 index 000000000000..9adb8c87b788 --- /dev/null +++ b/include/linux/firmware/intel/keembay_firmware.h @@ -0,0 +1,46 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Intel Keembay SOC Firmware API Layer + * + * Copyright (C) 2020-2021, Intel Corporation + * + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> + */ + +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ +#define __FIRMWARE_KEEMBAY_SMC_H__ + +#include <linux/arm-smccc.h> + +/** + * This file defines API function that can be called by device driver in order to + * communicate with Arm Trusted Firmware. + */ + +/* Setting for Keem Bay IO Pad Line Voltage Selection */ +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 +#define KEEMBAY_SET_1V8_VOLT 0x01 +#define KEEMBAY_SET_3V3_VOLT 0x00 + +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) +{ + struct arm_smccc_res res; + + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); + + return res.a0; +} + +int keembay_sd_voltage_selection(int volt) +{ + return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0); +} +#else +static inline int keembay_sd_voltage_selection(int volt) +{ + return -ENODEV; +} +#endif + +#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */ -- 2.17.1 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call @ 2020-10-06 15:55 ` muhammad.husaini.zulkifli 0 siblings, 0 replies; 41+ messages in thread From: muhammad.husaini.zulkifli @ 2020-10-06 15:55 UTC (permalink / raw) To: adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: lakshmi.bai.raja.subramanian, muhammad.husaini.zulkifli, arnd, wan.ahmad.zainie.wan.mohamad From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> Add header file to handle API function for device driver to communicate with Arm Trusted Firmware. Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> --- .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 include/linux/firmware/intel/keembay_firmware.h diff --git a/include/linux/firmware/intel/keembay_firmware.h b/include/linux/firmware/intel/keembay_firmware.h new file mode 100644 index 000000000000..9adb8c87b788 --- /dev/null +++ b/include/linux/firmware/intel/keembay_firmware.h @@ -0,0 +1,46 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Intel Keembay SOC Firmware API Layer + * + * Copyright (C) 2020-2021, Intel Corporation + * + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> + */ + +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ +#define __FIRMWARE_KEEMBAY_SMC_H__ + +#include <linux/arm-smccc.h> + +/** + * This file defines API function that can be called by device driver in order to + * communicate with Arm Trusted Firmware. + */ + +/* Setting for Keem Bay IO Pad Line Voltage Selection */ +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 +#define KEEMBAY_SET_1V8_VOLT 0x01 +#define KEEMBAY_SET_3V3_VOLT 0x00 + +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) +{ + struct arm_smccc_res res; + + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); + + return res.a0; +} + +int keembay_sd_voltage_selection(int volt) +{ + return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0); +} +#else +static inline int keembay_sd_voltage_selection(int volt) +{ + return -ENODEV; +} +#endif + +#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */ -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call 2020-10-06 15:55 ` muhammad.husaini.zulkifli (?) @ 2020-10-06 20:37 ` kernel test robot -1 siblings, 0 replies; 41+ messages in thread From: kernel test robot @ 2020-10-06 20:37 UTC (permalink / raw) To: muhammad.husaini.zulkifli, adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: kbuild-all, lakshmi.bai.raja.subramanian, wan.ahmad.zainie.wan.mohamad, muhammad.husaini.zulkifli [-- Attachment #1: Type: text/plain, Size: 2368 bytes --] Hi, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.9-rc8 next-20201006] [cannot apply to xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/muhammad-husaini-zulkifli-intel-com/mmc-sdhci-of-arasan-Enable-UHS-1-support-for-Keem-Bay-SOC/20201007-000542 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7575fdda569b2a2e8be32c1a64ecb05d6f96a500 config: arm-defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/273db2ef816560e1679afac43fd4e0624d5b7816 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review muhammad-husaini-zulkifli-intel-com/mmc-sdhci-of-arasan-Enable-UHS-1-support-for-Keem-Bay-SOC/20201007-000542 git checkout 273db2ef816560e1679afac43fd4e0624d5b7816 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): In file included from drivers/mmc/host/sdhci-of-arasan.c:27: >> include/linux/firmware/intel/keembay_firmware.h:35:5: warning: no previous prototype for 'keembay_sd_voltage_selection' [-Wmissing-prototypes] 35 | int keembay_sd_voltage_selection(int volt) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/keembay_sd_voltage_selection +35 include/linux/firmware/intel/keembay_firmware.h 34 > 35 int keembay_sd_voltage_selection(int volt) 36 { 37 return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0); 38 } 39 #else 40 static inline int keembay_sd_voltage_selection(int volt) 41 { 42 return -ENODEV; 43 } 44 #endif 45 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 53229 bytes --] ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call @ 2020-10-06 20:37 ` kernel test robot 0 siblings, 0 replies; 41+ messages in thread From: kernel test robot @ 2020-10-06 20:37 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 2423 bytes --] Hi, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.9-rc8 next-20201006] [cannot apply to xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/muhammad-husaini-zulkifli-intel-com/mmc-sdhci-of-arasan-Enable-UHS-1-support-for-Keem-Bay-SOC/20201007-000542 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7575fdda569b2a2e8be32c1a64ecb05d6f96a500 config: arm-defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/273db2ef816560e1679afac43fd4e0624d5b7816 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review muhammad-husaini-zulkifli-intel-com/mmc-sdhci-of-arasan-Enable-UHS-1-support-for-Keem-Bay-SOC/20201007-000542 git checkout 273db2ef816560e1679afac43fd4e0624d5b7816 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): In file included from drivers/mmc/host/sdhci-of-arasan.c:27: >> include/linux/firmware/intel/keembay_firmware.h:35:5: warning: no previous prototype for 'keembay_sd_voltage_selection' [-Wmissing-prototypes] 35 | int keembay_sd_voltage_selection(int volt) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/keembay_sd_voltage_selection +35 include/linux/firmware/intel/keembay_firmware.h 34 > 35 int keembay_sd_voltage_selection(int volt) 36 { 37 return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0); 38 } 39 #else 40 static inline int keembay_sd_voltage_selection(int volt) 41 { 42 return -ENODEV; 43 } 44 #endif 45 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 53229 bytes --] ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call @ 2020-10-06 20:37 ` kernel test robot 0 siblings, 0 replies; 41+ messages in thread From: kernel test robot @ 2020-10-06 20:37 UTC (permalink / raw) To: muhammad.husaini.zulkifli, adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: lakshmi.bai.raja.subramanian, muhammad.husaini.zulkifli, kbuild-all, wan.ahmad.zainie.wan.mohamad [-- Attachment #1: Type: text/plain, Size: 2368 bytes --] Hi, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.9-rc8 next-20201006] [cannot apply to xlnx/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/muhammad-husaini-zulkifli-intel-com/mmc-sdhci-of-arasan-Enable-UHS-1-support-for-Keem-Bay-SOC/20201007-000542 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 7575fdda569b2a2e8be32c1a64ecb05d6f96a500 config: arm-defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/273db2ef816560e1679afac43fd4e0624d5b7816 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review muhammad-husaini-zulkifli-intel-com/mmc-sdhci-of-arasan-Enable-UHS-1-support-for-Keem-Bay-SOC/20201007-000542 git checkout 273db2ef816560e1679afac43fd4e0624d5b7816 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): In file included from drivers/mmc/host/sdhci-of-arasan.c:27: >> include/linux/firmware/intel/keembay_firmware.h:35:5: warning: no previous prototype for 'keembay_sd_voltage_selection' [-Wmissing-prototypes] 35 | int keembay_sd_voltage_selection(int volt) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/keembay_sd_voltage_selection +35 include/linux/firmware/intel/keembay_firmware.h 34 > 35 int keembay_sd_voltage_selection(int volt) 36 { 37 return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0); 38 } 39 #else 40 static inline int keembay_sd_voltage_selection(int volt) 41 { 42 return -ENODEV; 43 } 44 #endif 45 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 53229 bytes --] [-- Attachment #3: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call 2020-10-06 15:55 ` muhammad.husaini.zulkifli @ 2020-10-07 3:41 ` Zulkifli, Muhammad Husaini -1 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 3:41 UTC (permalink / raw) To: Hunter, Adrian, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, Wan Mohamad, Wan Ahmad Zainie, arnd Hi Sudeep and Michal, I would like to receive some feedback on this patch before send out another version to fix the autobot compiler warning. Really appreciated it. Thanks >-----Original Message----- >From: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> >Sent: Tuesday, October 6, 2020 11:56 PM >To: Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >sudeep.holla@arm.com; ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; >linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org >Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >Wan Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; Zulkifli, Muhammad Husaini ><muhammad.husaini.zulkifli@intel.com>; arnd@arndb.de >Subject: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >Firmware Service call > >From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > >Add header file to handle API function for device driver to communicate with >Arm Trusted Firmware. > >Signed-off-by: Muhammad Husaini Zulkifli ><muhammad.husaini.zulkifli@intel.com> >--- > .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ > 1 file changed, 46 insertions(+) > create mode 100644 include/linux/firmware/intel/keembay_firmware.h > >diff --git a/include/linux/firmware/intel/keembay_firmware.h >b/include/linux/firmware/intel/keembay_firmware.h >new file mode 100644 >index 000000000000..9adb8c87b788 >--- /dev/null >+++ b/include/linux/firmware/intel/keembay_firmware.h >@@ -0,0 +1,46 @@ >+/* SPDX-License-Identifier: GPL-2.0 */ >+/* >+ * Intel Keembay SOC Firmware API Layer >+ * >+ * Copyright (C) 2020-2021, Intel Corporation >+ * >+ * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >+ */ >+ >+#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >+#define __FIRMWARE_KEEMBAY_SMC_H__ >+ >+#include <linux/arm-smccc.h> >+ >+/** >+ * This file defines API function that can be called by device driver >+in order to >+ * communicate with Arm Trusted Firmware. >+ */ >+ >+/* Setting for Keem Bay IO Pad Line Voltage Selection */ >+#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 >+#define KEEMBAY_SET_1V8_VOLT 0x01 >+#define KEEMBAY_SET_3V3_VOLT 0x00 >+ >+#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) >+static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) { >+ struct arm_smccc_res res; >+ >+ arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); >+ >+ return res.a0; >+} >+ >+int keembay_sd_voltage_selection(int volt) { >+ return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0); } >#else >+static inline int keembay_sd_voltage_selection(int volt) { >+ return -ENODEV; >+} >+#endif >+ >+#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */ >-- >2.17.1 ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call @ 2020-10-07 3:41 ` Zulkifli, Muhammad Husaini 0 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 3:41 UTC (permalink / raw) To: Hunter, Adrian, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, arnd, Wan Mohamad, Wan Ahmad Zainie Hi Sudeep and Michal, I would like to receive some feedback on this patch before send out another version to fix the autobot compiler warning. Really appreciated it. Thanks >-----Original Message----- >From: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com> >Sent: Tuesday, October 6, 2020 11:56 PM >To: Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >sudeep.holla@arm.com; ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; >linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org >Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >Wan Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; Zulkifli, Muhammad Husaini ><muhammad.husaini.zulkifli@intel.com>; arnd@arndb.de >Subject: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >Firmware Service call > >From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > >Add header file to handle API function for device driver to communicate with >Arm Trusted Firmware. > >Signed-off-by: Muhammad Husaini Zulkifli ><muhammad.husaini.zulkifli@intel.com> >--- > .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ > 1 file changed, 46 insertions(+) > create mode 100644 include/linux/firmware/intel/keembay_firmware.h > >diff --git a/include/linux/firmware/intel/keembay_firmware.h >b/include/linux/firmware/intel/keembay_firmware.h >new file mode 100644 >index 000000000000..9adb8c87b788 >--- /dev/null >+++ b/include/linux/firmware/intel/keembay_firmware.h >@@ -0,0 +1,46 @@ >+/* SPDX-License-Identifier: GPL-2.0 */ >+/* >+ * Intel Keembay SOC Firmware API Layer >+ * >+ * Copyright (C) 2020-2021, Intel Corporation >+ * >+ * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >+ */ >+ >+#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >+#define __FIRMWARE_KEEMBAY_SMC_H__ >+ >+#include <linux/arm-smccc.h> >+ >+/** >+ * This file defines API function that can be called by device driver >+in order to >+ * communicate with Arm Trusted Firmware. >+ */ >+ >+/* Setting for Keem Bay IO Pad Line Voltage Selection */ >+#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 >+#define KEEMBAY_SET_1V8_VOLT 0x01 >+#define KEEMBAY_SET_3V3_VOLT 0x00 >+ >+#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) >+static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) { >+ struct arm_smccc_res res; >+ >+ arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); >+ >+ return res.a0; >+} >+ >+int keembay_sd_voltage_selection(int volt) { >+ return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0); } >#else >+static inline int keembay_sd_voltage_selection(int volt) { >+ return -ENODEV; >+} >+#endif >+ >+#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */ >-- >2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call 2020-10-06 15:55 ` muhammad.husaini.zulkifli @ 2020-10-07 8:20 ` Michal Simek -1 siblings, 0 replies; 41+ messages in thread From: Michal Simek @ 2020-10-07 8:20 UTC (permalink / raw) To: muhammad.husaini.zulkifli, adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: lakshmi.bai.raja.subramanian, wan.ahmad.zainie.wan.mohamad, arnd Hi, 1. Keem Bay: in subject is wrong. Tools are working with it and you should just use keembay: instead. 2. This should come first before actual change to keep the tree bisectable. On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > > Add header file to handle API function for device driver to communicate > with Arm Trusted Firmware. > > Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > --- > .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ > 1 file changed, 46 insertions(+) > create mode 100644 include/linux/firmware/intel/keembay_firmware.h > > diff --git a/include/linux/firmware/intel/keembay_firmware.h b/include/linux/firmware/intel/keembay_firmware.h > new file mode 100644 > index 000000000000..9adb8c87b788 > --- /dev/null > +++ b/include/linux/firmware/intel/keembay_firmware.h > @@ -0,0 +1,46 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Intel Keembay SOC Firmware API Layer > + * > + * Copyright (C) 2020-2021, Intel Corporation > + * > + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> > + */ > + > +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ > +#define __FIRMWARE_KEEMBAY_SMC_H__ > + > +#include <linux/arm-smccc.h> > + > +/** This is not a kernel doc comment. Just use /* > + * This file defines API function that can be called by device driver in order to > + * communicate with Arm Trusted Firmware. > + */ > + > +/* Setting for Keem Bay IO Pad Line Voltage Selection */ > +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 Sudeep: Don't we have any macros for composing these IDs? nit: IMHO composing these IDs from macros would make more sense to me. > +#define KEEMBAY_SET_1V8_VOLT 0x01 0x01 is just 1 > +#define KEEMBAY_SET_3V3_VOLT 0x00 0x00 is just 0 > + > +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) > +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) > +{ > + struct arm_smccc_res res; > + > + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); > + > + return res.a0; I am not big fan of this error propagation in case of failure. If smc fails you get via res.a0 SMCCC_RET_NOT_SUPPORTED which is defined as -1 which is based on errno-base.h defined as EPERM. That driver which Sudeep pointed you to is using EINVAL instead. It means I would add a code to check it. > +} > + > +int keembay_sd_voltage_selection(int volt) as was reported by robot > +{ > + return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0); > +} > +#else > +static inline int keembay_sd_voltage_selection(int volt) > +{ > + return -ENODEV; > +} > +#endif > + > +#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */ > M ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call @ 2020-10-07 8:20 ` Michal Simek 0 siblings, 0 replies; 41+ messages in thread From: Michal Simek @ 2020-10-07 8:20 UTC (permalink / raw) To: muhammad.husaini.zulkifli, adrian.hunter, michal.simek, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: lakshmi.bai.raja.subramanian, arnd, wan.ahmad.zainie.wan.mohamad Hi, 1. Keem Bay: in subject is wrong. Tools are working with it and you should just use keembay: instead. 2. This should come first before actual change to keep the tree bisectable. On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > > Add header file to handle API function for device driver to communicate > with Arm Trusted Firmware. > > Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > --- > .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ > 1 file changed, 46 insertions(+) > create mode 100644 include/linux/firmware/intel/keembay_firmware.h > > diff --git a/include/linux/firmware/intel/keembay_firmware.h b/include/linux/firmware/intel/keembay_firmware.h > new file mode 100644 > index 000000000000..9adb8c87b788 > --- /dev/null > +++ b/include/linux/firmware/intel/keembay_firmware.h > @@ -0,0 +1,46 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Intel Keembay SOC Firmware API Layer > + * > + * Copyright (C) 2020-2021, Intel Corporation > + * > + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> > + */ > + > +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ > +#define __FIRMWARE_KEEMBAY_SMC_H__ > + > +#include <linux/arm-smccc.h> > + > +/** This is not a kernel doc comment. Just use /* > + * This file defines API function that can be called by device driver in order to > + * communicate with Arm Trusted Firmware. > + */ > + > +/* Setting for Keem Bay IO Pad Line Voltage Selection */ > +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 Sudeep: Don't we have any macros for composing these IDs? nit: IMHO composing these IDs from macros would make more sense to me. > +#define KEEMBAY_SET_1V8_VOLT 0x01 0x01 is just 1 > +#define KEEMBAY_SET_3V3_VOLT 0x00 0x00 is just 0 > + > +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) > +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) > +{ > + struct arm_smccc_res res; > + > + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); > + > + return res.a0; I am not big fan of this error propagation in case of failure. If smc fails you get via res.a0 SMCCC_RET_NOT_SUPPORTED which is defined as -1 which is based on errno-base.h defined as EPERM. That driver which Sudeep pointed you to is using EINVAL instead. It means I would add a code to check it. > +} > + > +int keembay_sd_voltage_selection(int volt) as was reported by robot > +{ > + return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0); > +} > +#else > +static inline int keembay_sd_voltage_selection(int volt) > +{ > + return -ENODEV; > +} > +#endif > + > +#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */ > M _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call 2020-10-07 8:20 ` Michal Simek @ 2020-10-07 10:10 ` Sudeep Holla -1 siblings, 0 replies; 41+ messages in thread From: Sudeep Holla @ 2020-10-07 10:10 UTC (permalink / raw) To: Michal Simek Cc: muhammad.husaini.zulkifli, adrian.hunter, Sudeep Holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel, lakshmi.bai.raja.subramanian, wan.ahmad.zainie.wan.mohamad, arnd On Wed, Oct 07, 2020 at 10:20:21AM +0200, Michal Simek wrote: > Hi, > > 1. Keem Bay: in subject is wrong. Tools are working with it and you > should just use keembay: instead. > > 2. This should come first before actual change to keep the tree bisectable. > > On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > > From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > > > > Add header file to handle API function for device driver to communicate > > with Arm Trusted Firmware. > > > > Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > > --- > > .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ > > 1 file changed, 46 insertions(+) > > create mode 100644 include/linux/firmware/intel/keembay_firmware.h > > > > diff --git a/include/linux/firmware/intel/keembay_firmware.h b/include/linux/firmware/intel/keembay_firmware.h > > new file mode 100644 > > index 000000000000..9adb8c87b788 > > --- /dev/null > > +++ b/include/linux/firmware/intel/keembay_firmware.h > > @@ -0,0 +1,46 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Intel Keembay SOC Firmware API Layer > > + * > > + * Copyright (C) 2020-2021, Intel Corporation > > + * > > + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> > > + */ > > + > > +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ > > +#define __FIRMWARE_KEEMBAY_SMC_H__ > > + > > +#include <linux/arm-smccc.h> > > + > > +/** > > This is not a kernel doc comment. Just use /* > > > + * This file defines API function that can be called by device driver in order to > > + * communicate with Arm Trusted Firmware. > > + */ > > + > > +/* Setting for Keem Bay IO Pad Line Voltage Selection */ > > +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 > > Sudeep: Don't we have any macros for composing these IDs? > nit: IMHO composing these IDs from macros would make more sense to me. > Yes we do. Refer include/linux/arm-smccc.h I expect something like below, which also indicated you are using wrong OWNER space. You can't be 0 which is reserved for CPU ARCH. You need to be SIP(0x2) #define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE \ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ ARM_SMCCC_SMC_32, \ ARM_SMCCC_OWNER_SIP, \ 0xFF26) -- Regards, Sudeep ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call @ 2020-10-07 10:10 ` Sudeep Holla 0 siblings, 0 replies; 41+ messages in thread From: Sudeep Holla @ 2020-10-07 10:10 UTC (permalink / raw) To: Michal Simek Cc: ulf.hansson, arnd, lakshmi.bai.raja.subramanian, linux-mmc, adrian.hunter, linux-kernel, wan.ahmad.zainie.wan.mohamad, Sudeep Holla, muhammad.husaini.zulkifli, linux-arm-kernel On Wed, Oct 07, 2020 at 10:20:21AM +0200, Michal Simek wrote: > Hi, > > 1. Keem Bay: in subject is wrong. Tools are working with it and you > should just use keembay: instead. > > 2. This should come first before actual change to keep the tree bisectable. > > On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: > > From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > > > > Add header file to handle API function for device driver to communicate > > with Arm Trusted Firmware. > > > > Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> > > --- > > .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ > > 1 file changed, 46 insertions(+) > > create mode 100644 include/linux/firmware/intel/keembay_firmware.h > > > > diff --git a/include/linux/firmware/intel/keembay_firmware.h b/include/linux/firmware/intel/keembay_firmware.h > > new file mode 100644 > > index 000000000000..9adb8c87b788 > > --- /dev/null > > +++ b/include/linux/firmware/intel/keembay_firmware.h > > @@ -0,0 +1,46 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Intel Keembay SOC Firmware API Layer > > + * > > + * Copyright (C) 2020-2021, Intel Corporation > > + * > > + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> > > + */ > > + > > +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ > > +#define __FIRMWARE_KEEMBAY_SMC_H__ > > + > > +#include <linux/arm-smccc.h> > > + > > +/** > > This is not a kernel doc comment. Just use /* > > > + * This file defines API function that can be called by device driver in order to > > + * communicate with Arm Trusted Firmware. > > + */ > > + > > +/* Setting for Keem Bay IO Pad Line Voltage Selection */ > > +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 > > Sudeep: Don't we have any macros for composing these IDs? > nit: IMHO composing these IDs from macros would make more sense to me. > Yes we do. Refer include/linux/arm-smccc.h I expect something like below, which also indicated you are using wrong OWNER space. You can't be 0 which is reserved for CPU ARCH. You need to be SIP(0x2) #define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE \ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ ARM_SMCCC_SMC_32, \ ARM_SMCCC_OWNER_SIP, \ 0xFF26) -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call 2020-10-07 10:10 ` Sudeep Holla @ 2020-10-07 13:29 ` Zulkifli, Muhammad Husaini -1 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 13:29 UTC (permalink / raw) To: Sudeep Holla, Michal Simek Cc: Hunter, Adrian, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel, Raja Subramanian, Lakshmi Bai, Wan Mohamad, Wan Ahmad Zainie, arnd Hi Sudeep, Thanks for the feedback. I replied inline >-----Original Message----- >From: Sudeep Holla <sudeep.holla@arm.com> >Sent: Wednesday, October 7, 2020 6:11 PM >To: Michal Simek <michal.simek@xilinx.com> >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >Hunter, Adrian <adrian.hunter@intel.com>; Sudeep Holla ><sudeep.holla@arm.com>; ulf.hansson@linaro.org; linux- >mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- >kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai ><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan Ahmad >Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >Firmware Service call > >On Wed, Oct 07, 2020 at 10:20:21AM +0200, Michal Simek wrote: >> Hi, >> >> 1. Keem Bay: in subject is wrong. Tools are working with it and you >> should just use keembay: instead. >> >> 2. This should come first before actual change to keep the tree bisectable. >> >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >> > From: Muhammad Husaini Zulkifli >> > <muhammad.husaini.zulkifli@intel.com> >> > >> > Add header file to handle API function for device driver to >> > communicate with Arm Trusted Firmware. >> > >> > Signed-off-by: Muhammad Husaini Zulkifli >> > <muhammad.husaini.zulkifli@intel.com> >> > --- >> > .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ >> > 1 file changed, 46 insertions(+) >> > create mode 100644 include/linux/firmware/intel/keembay_firmware.h >> > >> > diff --git a/include/linux/firmware/intel/keembay_firmware.h >> > b/include/linux/firmware/intel/keembay_firmware.h >> > new file mode 100644 >> > index 000000000000..9adb8c87b788 >> > --- /dev/null >> > +++ b/include/linux/firmware/intel/keembay_firmware.h >> > @@ -0,0 +1,46 @@ >> > +/* SPDX-License-Identifier: GPL-2.0 */ >> > +/* >> > + * Intel Keembay SOC Firmware API Layer >> > + * >> > + * Copyright (C) 2020-2021, Intel Corporation >> > + * >> > + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >> > + */ >> > + >> > +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >> > +#define __FIRMWARE_KEEMBAY_SMC_H__ >> > + >> > +#include <linux/arm-smccc.h> >> > + >> > +/** >> >> This is not a kernel doc comment. Just use /* >> >> > + * This file defines API function that can be called by device >> > + driver in order to >> > + * communicate with Arm Trusted Firmware. >> > + */ >> > + >> > +/* Setting for Keem Bay IO Pad Line Voltage Selection */ >> > +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 >> >> Sudeep: Don't we have any macros for composing these IDs? >> nit: IMHO composing these IDs from macros would make more sense to me. >> > >Yes we do. Refer include/linux/arm-smccc.h I expect something like below, >which also indicated you are using wrong OWNER space. You can't be 0 which is >reserved for CPU ARCH. You need to be SIP(0x2) > >#define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE \ > ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > ARM_SMCCC_SMC_32, \ > ARM_SMCCC_OWNER_SIP, \ > 0xFF26) > > >-- Testing with below func and definition. It is working . #define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE \ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ ARM_SMCCC_SMC_32, \ ARM_SMCCC_OWNER_SIP, \ 0xFF26) int keembay_sd_voltage_selection(int volt) { struct arm_smccc_res res; arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE, volt, &res); if ((int)res.a0 < 0) return -EINVAL; return 0; } >Regards, >Sudeep ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call @ 2020-10-07 13:29 ` Zulkifli, Muhammad Husaini 0 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 13:29 UTC (permalink / raw) To: Sudeep Holla, Michal Simek Cc: ulf.hansson, arnd, Raja Subramanian, Lakshmi Bai, linux-mmc, Hunter, Adrian, linux-kernel, Wan Mohamad, Wan Ahmad Zainie, linux-arm-kernel Hi Sudeep, Thanks for the feedback. I replied inline >-----Original Message----- >From: Sudeep Holla <sudeep.holla@arm.com> >Sent: Wednesday, October 7, 2020 6:11 PM >To: Michal Simek <michal.simek@xilinx.com> >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >Hunter, Adrian <adrian.hunter@intel.com>; Sudeep Holla ><sudeep.holla@arm.com>; ulf.hansson@linaro.org; linux- >mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- >kernel@vger.kernel.org; Raja Subramanian, Lakshmi Bai ><lakshmi.bai.raja.subramanian@intel.com>; Wan Mohamad, Wan Ahmad >Zainie <wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >Firmware Service call > >On Wed, Oct 07, 2020 at 10:20:21AM +0200, Michal Simek wrote: >> Hi, >> >> 1. Keem Bay: in subject is wrong. Tools are working with it and you >> should just use keembay: instead. >> >> 2. This should come first before actual change to keep the tree bisectable. >> >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >> > From: Muhammad Husaini Zulkifli >> > <muhammad.husaini.zulkifli@intel.com> >> > >> > Add header file to handle API function for device driver to >> > communicate with Arm Trusted Firmware. >> > >> > Signed-off-by: Muhammad Husaini Zulkifli >> > <muhammad.husaini.zulkifli@intel.com> >> > --- >> > .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ >> > 1 file changed, 46 insertions(+) >> > create mode 100644 include/linux/firmware/intel/keembay_firmware.h >> > >> > diff --git a/include/linux/firmware/intel/keembay_firmware.h >> > b/include/linux/firmware/intel/keembay_firmware.h >> > new file mode 100644 >> > index 000000000000..9adb8c87b788 >> > --- /dev/null >> > +++ b/include/linux/firmware/intel/keembay_firmware.h >> > @@ -0,0 +1,46 @@ >> > +/* SPDX-License-Identifier: GPL-2.0 */ >> > +/* >> > + * Intel Keembay SOC Firmware API Layer >> > + * >> > + * Copyright (C) 2020-2021, Intel Corporation >> > + * >> > + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >> > + */ >> > + >> > +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >> > +#define __FIRMWARE_KEEMBAY_SMC_H__ >> > + >> > +#include <linux/arm-smccc.h> >> > + >> > +/** >> >> This is not a kernel doc comment. Just use /* >> >> > + * This file defines API function that can be called by device >> > + driver in order to >> > + * communicate with Arm Trusted Firmware. >> > + */ >> > + >> > +/* Setting for Keem Bay IO Pad Line Voltage Selection */ >> > +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 >> >> Sudeep: Don't we have any macros for composing these IDs? >> nit: IMHO composing these IDs from macros would make more sense to me. >> > >Yes we do. Refer include/linux/arm-smccc.h I expect something like below, >which also indicated you are using wrong OWNER space. You can't be 0 which is >reserved for CPU ARCH. You need to be SIP(0x2) > >#define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE \ > ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > ARM_SMCCC_SMC_32, \ > ARM_SMCCC_OWNER_SIP, \ > 0xFF26) > > >-- Testing with below func and definition. It is working . #define ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE \ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ ARM_SMCCC_SMC_32, \ ARM_SMCCC_OWNER_SIP, \ 0xFF26) int keembay_sd_voltage_selection(int volt) { struct arm_smccc_res res; arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE, volt, &res); if ((int)res.a0 < 0) return -EINVAL; return 0; } >Regards, >Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call 2020-10-07 8:20 ` Michal Simek @ 2020-10-07 13:21 ` Zulkifli, Muhammad Husaini -1 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 13:21 UTC (permalink / raw) To: Michal Simek, Hunter, Adrian, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, Wan Mohamad, Wan Ahmad Zainie, arnd Hi Michal, Thanks for the feedback. I replied inline >-----Original Message----- >From: Michal Simek <michal.simek@xilinx.com> >Sent: Wednesday, October 7, 2020 4:20 PM >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >sudeep.holla@arm.com; ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; >linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org >Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >Wan Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >Firmware Service call > >Hi, > >1. Keem Bay: in subject is wrong. Tools are working with it and you should just >use keembay: instead. Are you saying like this ? Keem Bay: Add support for Arm Trusted Firmware Service call > >2. This should come first before actual change to keep the tree bisectable. Noted. Done the changes > >On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> >> >> Add header file to handle API function for device driver to >> communicate with Arm Trusted Firmware. >> >> Signed-off-by: Muhammad Husaini Zulkifli >> <muhammad.husaini.zulkifli@intel.com> >> --- >> .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ >> 1 file changed, 46 insertions(+) >> create mode 100644 include/linux/firmware/intel/keembay_firmware.h >> >> diff --git a/include/linux/firmware/intel/keembay_firmware.h >> b/include/linux/firmware/intel/keembay_firmware.h >> new file mode 100644 >> index 000000000000..9adb8c87b788 >> --- /dev/null >> +++ b/include/linux/firmware/intel/keembay_firmware.h >> @@ -0,0 +1,46 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * Intel Keembay SOC Firmware API Layer >> + * >> + * Copyright (C) 2020-2021, Intel Corporation >> + * >> + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >> + */ >> + >> +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >> +#define __FIRMWARE_KEEMBAY_SMC_H__ >> + >> +#include <linux/arm-smccc.h> >> + >> +/** > >This is not a kernel doc comment. Just use /* > >> + * This file defines API function that can be called by device driver >> + in order to >> + * communicate with Arm Trusted Firmware. >> + */ >> + >> +/* Setting for Keem Bay IO Pad Line Voltage Selection */ >> +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 > >Sudeep: Don't we have any macros for composing these IDs? >nit: IMHO composing these IDs from macros would make more sense to me. > > >> +#define KEEMBAY_SET_1V8_VOLT 0x01 > >0x01 is just 1 Noted. Done the changes > >> +#define KEEMBAY_SET_3V3_VOLT 0x00 > >0x00 is just 0 Noted. Done the changes > >> + >> +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) >> +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) { >> + struct arm_smccc_res res; >> + >> + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); >> + >> + return res.a0; > >I am not big fan of this error propagation in case of failure. > >If smc fails you get via res.a0 SMCCC_RET_NOT_SUPPORTED which is defined as >-1 which is based on errno-base.h defined as EPERM. > >That driver which Sudeep pointed you to is using EINVAL instead. > >It means I would add a code to check it. Yeah I changed to below line of codes. Is this Ok? Tested working. int keembay_sd_voltage_selection(int volt) { struct arm_smccc_res res; arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE, volt, &res); if ((int)res.a0 < 0) return -EINVAL; return 0; } > > >> +} >> + >> +int keembay_sd_voltage_selection(int volt) > >as was reported by robot Added the func prototype. int keembay_sd_voltage_selection(int volt); No error observed after that. > >> +{ >> + return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0); } >> +#else static inline int keembay_sd_voltage_selection(int volt) { >> + return -ENODEV; >> +} >> +#endif >> + >> +#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */ >> > >M ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call @ 2020-10-07 13:21 ` Zulkifli, Muhammad Husaini 0 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 13:21 UTC (permalink / raw) To: Michal Simek, Hunter, Adrian, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, arnd, Wan Mohamad, Wan Ahmad Zainie Hi Michal, Thanks for the feedback. I replied inline >-----Original Message----- >From: Michal Simek <michal.simek@xilinx.com> >Sent: Wednesday, October 7, 2020 4:20 PM >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >sudeep.holla@arm.com; ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; >linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org >Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >Wan Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >Firmware Service call > >Hi, > >1. Keem Bay: in subject is wrong. Tools are working with it and you should just >use keembay: instead. Are you saying like this ? Keem Bay: Add support for Arm Trusted Firmware Service call > >2. This should come first before actual change to keep the tree bisectable. Noted. Done the changes > >On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> >> >> Add header file to handle API function for device driver to >> communicate with Arm Trusted Firmware. >> >> Signed-off-by: Muhammad Husaini Zulkifli >> <muhammad.husaini.zulkifli@intel.com> >> --- >> .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ >> 1 file changed, 46 insertions(+) >> create mode 100644 include/linux/firmware/intel/keembay_firmware.h >> >> diff --git a/include/linux/firmware/intel/keembay_firmware.h >> b/include/linux/firmware/intel/keembay_firmware.h >> new file mode 100644 >> index 000000000000..9adb8c87b788 >> --- /dev/null >> +++ b/include/linux/firmware/intel/keembay_firmware.h >> @@ -0,0 +1,46 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * Intel Keembay SOC Firmware API Layer >> + * >> + * Copyright (C) 2020-2021, Intel Corporation >> + * >> + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >> + */ >> + >> +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >> +#define __FIRMWARE_KEEMBAY_SMC_H__ >> + >> +#include <linux/arm-smccc.h> >> + >> +/** > >This is not a kernel doc comment. Just use /* > >> + * This file defines API function that can be called by device driver >> + in order to >> + * communicate with Arm Trusted Firmware. >> + */ >> + >> +/* Setting for Keem Bay IO Pad Line Voltage Selection */ >> +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 > >Sudeep: Don't we have any macros for composing these IDs? >nit: IMHO composing these IDs from macros would make more sense to me. > > >> +#define KEEMBAY_SET_1V8_VOLT 0x01 > >0x01 is just 1 Noted. Done the changes > >> +#define KEEMBAY_SET_3V3_VOLT 0x00 > >0x00 is just 0 Noted. Done the changes > >> + >> +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) >> +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) { >> + struct arm_smccc_res res; >> + >> + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); >> + >> + return res.a0; > >I am not big fan of this error propagation in case of failure. > >If smc fails you get via res.a0 SMCCC_RET_NOT_SUPPORTED which is defined as >-1 which is based on errno-base.h defined as EPERM. > >That driver which Sudeep pointed you to is using EINVAL instead. > >It means I would add a code to check it. Yeah I changed to below line of codes. Is this Ok? Tested working. int keembay_sd_voltage_selection(int volt) { struct arm_smccc_res res; arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE, volt, &res); if ((int)res.a0 < 0) return -EINVAL; return 0; } > > >> +} >> + >> +int keembay_sd_voltage_selection(int volt) > >as was reported by robot Added the func prototype. int keembay_sd_voltage_selection(int volt); No error observed after that. > >> +{ >> + return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0); } >> +#else static inline int keembay_sd_voltage_selection(int volt) { >> + return -ENODEV; >> +} >> +#endif >> + >> +#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */ >> > >M _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call 2020-10-07 13:21 ` Zulkifli, Muhammad Husaini @ 2020-10-07 13:37 ` Michal Simek -1 siblings, 0 replies; 41+ messages in thread From: Michal Simek @ 2020-10-07 13:37 UTC (permalink / raw) To: Zulkifli, Muhammad Husaini, Michal Simek, Hunter, Adrian, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, Wan Mohamad, Wan Ahmad Zainie, arnd Hi, On 07. 10. 20 15:21, Zulkifli, Muhammad Husaini wrote: > Hi Michal, > > Thanks for the feedback. I replied inline > >> -----Original Message----- >> From: Michal Simek <michal.simek@xilinx.com> >> Sent: Wednesday, October 7, 2020 4:20 PM >> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >> Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >> sudeep.holla@arm.com; ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; >> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org >> Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >> Wan Mohamad, Wan Ahmad Zainie >> <wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >> Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >> Firmware Service call >> >> Hi, >> >> 1. Keem Bay: in subject is wrong. Tools are working with it and you should just >> use keembay: instead. > Are you saying like this ? > Keem Bay: Add support for Arm Trusted Firmware Service call like this: firmware: keembay: Add support for Arm Trusted Firmware Service call > >> >> 2. This should come first before actual change to keep the tree bisectable. > Noted. Done the changes >> >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >>> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> >>> >>> Add header file to handle API function for device driver to >>> communicate with Arm Trusted Firmware. >>> >>> Signed-off-by: Muhammad Husaini Zulkifli >>> <muhammad.husaini.zulkifli@intel.com> >>> --- >>> .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ >>> 1 file changed, 46 insertions(+) >>> create mode 100644 include/linux/firmware/intel/keembay_firmware.h >>> >>> diff --git a/include/linux/firmware/intel/keembay_firmware.h >>> b/include/linux/firmware/intel/keembay_firmware.h >>> new file mode 100644 >>> index 000000000000..9adb8c87b788 >>> --- /dev/null >>> +++ b/include/linux/firmware/intel/keembay_firmware.h >>> @@ -0,0 +1,46 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +/* >>> + * Intel Keembay SOC Firmware API Layer >>> + * >>> + * Copyright (C) 2020-2021, Intel Corporation >>> + * >>> + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >>> + */ >>> + >>> +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >>> +#define __FIRMWARE_KEEMBAY_SMC_H__ >>> + >>> +#include <linux/arm-smccc.h> >>> + >>> +/** >> >> This is not a kernel doc comment. Just use /* >> >>> + * This file defines API function that can be called by device driver >>> + in order to >>> + * communicate with Arm Trusted Firmware. >>> + */ >>> + >>> +/* Setting for Keem Bay IO Pad Line Voltage Selection */ >>> +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 >> >> Sudeep: Don't we have any macros for composing these IDs? >> nit: IMHO composing these IDs from macros would make more sense to me. >> >> >>> +#define KEEMBAY_SET_1V8_VOLT 0x01 >> >> 0x01 is just 1 > Noted. Done the changes >> >>> +#define KEEMBAY_SET_3V3_VOLT 0x00 >> >> 0x00 is just 0 > Noted. Done the changes >> >>> + >>> +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) >>> +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) { >>> + struct arm_smccc_res res; >>> + >>> + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); >>> + >>> + return res.a0; >> >> I am not big fan of this error propagation in case of failure. >> >> If smc fails you get via res.a0 SMCCC_RET_NOT_SUPPORTED which is defined as >> -1 which is based on errno-base.h defined as EPERM. >> >> That driver which Sudeep pointed you to is using EINVAL instead. >> >> It means I would add a code to check it. > > Yeah I changed to below line of codes. Is this Ok? Tested working. > int keembay_sd_voltage_selection(int volt) static inline here shouldn't hurt. > { > struct arm_smccc_res res; > > arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE, volt, &res); > if ((int)res.a0 < 0) > return -EINVAL; > > return 0; > } This is fine. M ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call @ 2020-10-07 13:37 ` Michal Simek 0 siblings, 0 replies; 41+ messages in thread From: Michal Simek @ 2020-10-07 13:37 UTC (permalink / raw) To: Zulkifli, Muhammad Husaini, Michal Simek, Hunter, Adrian, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, arnd, Wan Mohamad, Wan Ahmad Zainie Hi, On 07. 10. 20 15:21, Zulkifli, Muhammad Husaini wrote: > Hi Michal, > > Thanks for the feedback. I replied inline > >> -----Original Message----- >> From: Michal Simek <michal.simek@xilinx.com> >> Sent: Wednesday, October 7, 2020 4:20 PM >> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >> Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >> sudeep.holla@arm.com; ulf.hansson@linaro.org; linux-mmc@vger.kernel.org; >> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org >> Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >> Wan Mohamad, Wan Ahmad Zainie >> <wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >> Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >> Firmware Service call >> >> Hi, >> >> 1. Keem Bay: in subject is wrong. Tools are working with it and you should just >> use keembay: instead. > Are you saying like this ? > Keem Bay: Add support for Arm Trusted Firmware Service call like this: firmware: keembay: Add support for Arm Trusted Firmware Service call > >> >> 2. This should come first before actual change to keep the tree bisectable. > Noted. Done the changes >> >> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >>> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com> >>> >>> Add header file to handle API function for device driver to >>> communicate with Arm Trusted Firmware. >>> >>> Signed-off-by: Muhammad Husaini Zulkifli >>> <muhammad.husaini.zulkifli@intel.com> >>> --- >>> .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ >>> 1 file changed, 46 insertions(+) >>> create mode 100644 include/linux/firmware/intel/keembay_firmware.h >>> >>> diff --git a/include/linux/firmware/intel/keembay_firmware.h >>> b/include/linux/firmware/intel/keembay_firmware.h >>> new file mode 100644 >>> index 000000000000..9adb8c87b788 >>> --- /dev/null >>> +++ b/include/linux/firmware/intel/keembay_firmware.h >>> @@ -0,0 +1,46 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >>> +/* >>> + * Intel Keembay SOC Firmware API Layer >>> + * >>> + * Copyright (C) 2020-2021, Intel Corporation >>> + * >>> + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >>> + */ >>> + >>> +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >>> +#define __FIRMWARE_KEEMBAY_SMC_H__ >>> + >>> +#include <linux/arm-smccc.h> >>> + >>> +/** >> >> This is not a kernel doc comment. Just use /* >> >>> + * This file defines API function that can be called by device driver >>> + in order to >>> + * communicate with Arm Trusted Firmware. >>> + */ >>> + >>> +/* Setting for Keem Bay IO Pad Line Voltage Selection */ >>> +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 >> >> Sudeep: Don't we have any macros for composing these IDs? >> nit: IMHO composing these IDs from macros would make more sense to me. >> >> >>> +#define KEEMBAY_SET_1V8_VOLT 0x01 >> >> 0x01 is just 1 > Noted. Done the changes >> >>> +#define KEEMBAY_SET_3V3_VOLT 0x00 >> >> 0x00 is just 0 > Noted. Done the changes >> >>> + >>> +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) >>> +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) { >>> + struct arm_smccc_res res; >>> + >>> + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); >>> + >>> + return res.a0; >> >> I am not big fan of this error propagation in case of failure. >> >> If smc fails you get via res.a0 SMCCC_RET_NOT_SUPPORTED which is defined as >> -1 which is based on errno-base.h defined as EPERM. >> >> That driver which Sudeep pointed you to is using EINVAL instead. >> >> It means I would add a code to check it. > > Yeah I changed to below line of codes. Is this Ok? Tested working. > int keembay_sd_voltage_selection(int volt) static inline here shouldn't hurt. > { > struct arm_smccc_res res; > > arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAGE, volt, &res); > if ((int)res.a0 < 0) > return -EINVAL; > > return 0; > } This is fine. M _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call 2020-10-07 13:37 ` Michal Simek @ 2020-10-07 13:52 ` Zulkifli, Muhammad Husaini -1 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 13:52 UTC (permalink / raw) To: Michal Simek, Hunter, Adrian, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, Wan Mohamad, Wan Ahmad Zainie, arnd Hi, >-----Original Message----- >From: Michal Simek <michal.simek@xilinx.com> >Sent: Wednesday, October 7, 2020 9:37 PM >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian ><adrian.hunter@intel.com>; sudeep.holla@arm.com; ulf.hansson@linaro.org; >linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- >kernel@vger.kernel.org >Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >Wan Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >Firmware Service call > >Hi, > >On 07. 10. 20 15:21, Zulkifli, Muhammad Husaini wrote: >> Hi Michal, >> >> Thanks for the feedback. I replied inline >> >>> -----Original Message----- >>> From: Michal Simek <michal.simek@xilinx.com> >>> Sent: Wednesday, October 7, 2020 4:20 PM >>> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >>> Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >>> sudeep.holla@arm.com; ulf.hansson@linaro.org; >>> linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >>> linux-kernel@vger.kernel.org >>> Cc: Raja Subramanian, Lakshmi Bai >>> <lakshmi.bai.raja.subramanian@intel.com>; >>> Wan Mohamad, Wan Ahmad Zainie >>> <wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >>> Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm >>> Trusted Firmware Service call >>> >>> Hi, >>> >>> 1. Keem Bay: in subject is wrong. Tools are working with it and you >>> should just use keembay: instead. >> Are you saying like this ? >> Keem Bay: Add support for Arm Trusted Firmware Service call > >like this: >firmware: keembay: Add support for Arm Trusted Firmware Service call > >> >>> >>> 2. This should come first before actual change to keep the tree bisectable. >> Noted. Done the changes >>> >>> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >>>> From: Muhammad Husaini Zulkifli >>>> <muhammad.husaini.zulkifli@intel.com> >>>> >>>> Add header file to handle API function for device driver to >>>> communicate with Arm Trusted Firmware. >>>> >>>> Signed-off-by: Muhammad Husaini Zulkifli >>>> <muhammad.husaini.zulkifli@intel.com> >>>> --- >>>> .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ >>>> 1 file changed, 46 insertions(+) >>>> create mode 100644 include/linux/firmware/intel/keembay_firmware.h >>>> >>>> diff --git a/include/linux/firmware/intel/keembay_firmware.h >>>> b/include/linux/firmware/intel/keembay_firmware.h >>>> new file mode 100644 >>>> index 000000000000..9adb8c87b788 >>>> --- /dev/null >>>> +++ b/include/linux/firmware/intel/keembay_firmware.h >>>> @@ -0,0 +1,46 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>> +/* >>>> + * Intel Keembay SOC Firmware API Layer >>>> + * >>>> + * Copyright (C) 2020-2021, Intel Corporation >>>> + * >>>> + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >>>> + */ >>>> + >>>> +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >>>> +#define __FIRMWARE_KEEMBAY_SMC_H__ >>>> + >>>> +#include <linux/arm-smccc.h> >>>> + >>>> +/** >>> >>> This is not a kernel doc comment. Just use /* >>> >>>> + * This file defines API function that can be called by device >>>> + driver in order to >>>> + * communicate with Arm Trusted Firmware. >>>> + */ >>>> + >>>> +/* Setting for Keem Bay IO Pad Line Voltage Selection */ >>>> +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 >>> >>> Sudeep: Don't we have any macros for composing these IDs? >>> nit: IMHO composing these IDs from macros would make more sense to me. >>> >>> >>>> +#define KEEMBAY_SET_1V8_VOLT 0x01 >>> >>> 0x01 is just 1 >> Noted. Done the changes >>> >>>> +#define KEEMBAY_SET_3V3_VOLT 0x00 >>> >>> 0x00 is just 0 >> Noted. Done the changes >>> >>>> + >>>> +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) >>>> +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) { >>>> + struct arm_smccc_res res; >>>> + >>>> + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); >>>> + >>>> + return res.a0; >>> >>> I am not big fan of this error propagation in case of failure. >>> >>> If smc fails you get via res.a0 SMCCC_RET_NOT_SUPPORTED which is >>> defined as >>> -1 which is based on errno-base.h defined as EPERM. >>> >>> That driver which Sudeep pointed you to is using EINVAL instead. >>> >>> It means I would add a code to check it. >> >> Yeah I changed to below line of codes. Is this Ok? Tested working. >> int keembay_sd_voltage_selection(int volt) > >static inline here shouldn't hurt. due to func() prototype " int keembay_sd_voltage_selection(int volt);" to solve warning issues by robot , I cannot set static inline here. Will observed below error: error: static declaration of ‘keembay_sd_voltage_selection’ follows non-static declaration static inline int keembay_sd_voltage_selection(int volt). > >> { >> struct arm_smccc_res res; >> >> > arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAG >E, volt, &res); >> if ((int)res.a0 < 0) >> return -EINVAL; >> >> return 0; >> } > >This is fine. > >M ^ permalink raw reply [flat|nested] 41+ messages in thread
* RE: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call @ 2020-10-07 13:52 ` Zulkifli, Muhammad Husaini 0 siblings, 0 replies; 41+ messages in thread From: Zulkifli, Muhammad Husaini @ 2020-10-07 13:52 UTC (permalink / raw) To: Michal Simek, Hunter, Adrian, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, arnd, Wan Mohamad, Wan Ahmad Zainie Hi, >-----Original Message----- >From: Michal Simek <michal.simek@xilinx.com> >Sent: Wednesday, October 7, 2020 9:37 PM >To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian ><adrian.hunter@intel.com>; sudeep.holla@arm.com; ulf.hansson@linaro.org; >linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- >kernel@vger.kernel.org >Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >Wan Mohamad, Wan Ahmad Zainie ><wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >Firmware Service call > >Hi, > >On 07. 10. 20 15:21, Zulkifli, Muhammad Husaini wrote: >> Hi Michal, >> >> Thanks for the feedback. I replied inline >> >>> -----Original Message----- >>> From: Michal Simek <michal.simek@xilinx.com> >>> Sent: Wednesday, October 7, 2020 4:20 PM >>> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >>> Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >>> sudeep.holla@arm.com; ulf.hansson@linaro.org; >>> linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >>> linux-kernel@vger.kernel.org >>> Cc: Raja Subramanian, Lakshmi Bai >>> <lakshmi.bai.raja.subramanian@intel.com>; >>> Wan Mohamad, Wan Ahmad Zainie >>> <wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >>> Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm >>> Trusted Firmware Service call >>> >>> Hi, >>> >>> 1. Keem Bay: in subject is wrong. Tools are working with it and you >>> should just use keembay: instead. >> Are you saying like this ? >> Keem Bay: Add support for Arm Trusted Firmware Service call > >like this: >firmware: keembay: Add support for Arm Trusted Firmware Service call > >> >>> >>> 2. This should come first before actual change to keep the tree bisectable. >> Noted. Done the changes >>> >>> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >>>> From: Muhammad Husaini Zulkifli >>>> <muhammad.husaini.zulkifli@intel.com> >>>> >>>> Add header file to handle API function for device driver to >>>> communicate with Arm Trusted Firmware. >>>> >>>> Signed-off-by: Muhammad Husaini Zulkifli >>>> <muhammad.husaini.zulkifli@intel.com> >>>> --- >>>> .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ >>>> 1 file changed, 46 insertions(+) >>>> create mode 100644 include/linux/firmware/intel/keembay_firmware.h >>>> >>>> diff --git a/include/linux/firmware/intel/keembay_firmware.h >>>> b/include/linux/firmware/intel/keembay_firmware.h >>>> new file mode 100644 >>>> index 000000000000..9adb8c87b788 >>>> --- /dev/null >>>> +++ b/include/linux/firmware/intel/keembay_firmware.h >>>> @@ -0,0 +1,46 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>> +/* >>>> + * Intel Keembay SOC Firmware API Layer >>>> + * >>>> + * Copyright (C) 2020-2021, Intel Corporation >>>> + * >>>> + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >>>> + */ >>>> + >>>> +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >>>> +#define __FIRMWARE_KEEMBAY_SMC_H__ >>>> + >>>> +#include <linux/arm-smccc.h> >>>> + >>>> +/** >>> >>> This is not a kernel doc comment. Just use /* >>> >>>> + * This file defines API function that can be called by device >>>> + driver in order to >>>> + * communicate with Arm Trusted Firmware. >>>> + */ >>>> + >>>> +/* Setting for Keem Bay IO Pad Line Voltage Selection */ >>>> +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 >>> >>> Sudeep: Don't we have any macros for composing these IDs? >>> nit: IMHO composing these IDs from macros would make more sense to me. >>> >>> >>>> +#define KEEMBAY_SET_1V8_VOLT 0x01 >>> >>> 0x01 is just 1 >> Noted. Done the changes >>> >>>> +#define KEEMBAY_SET_3V3_VOLT 0x00 >>> >>> 0x00 is just 0 >> Noted. Done the changes >>> >>>> + >>>> +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) >>>> +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) { >>>> + struct arm_smccc_res res; >>>> + >>>> + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); >>>> + >>>> + return res.a0; >>> >>> I am not big fan of this error propagation in case of failure. >>> >>> If smc fails you get via res.a0 SMCCC_RET_NOT_SUPPORTED which is >>> defined as >>> -1 which is based on errno-base.h defined as EPERM. >>> >>> That driver which Sudeep pointed you to is using EINVAL instead. >>> >>> It means I would add a code to check it. >> >> Yeah I changed to below line of codes. Is this Ok? Tested working. >> int keembay_sd_voltage_selection(int volt) > >static inline here shouldn't hurt. due to func() prototype " int keembay_sd_voltage_selection(int volt);" to solve warning issues by robot , I cannot set static inline here. Will observed below error: error: static declaration of ‘keembay_sd_voltage_selection’ follows non-static declaration static inline int keembay_sd_voltage_selection(int volt). > >> { >> struct arm_smccc_res res; >> >> > arm_smccc_1_1_invoke(ARM_SMCCC_SIP_KEEMBAY_SET_SD_VOLTAG >E, volt, &res); >> if ((int)res.a0 < 0) >> return -EINVAL; >> >> return 0; >> } > >This is fine. > >M _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call 2020-10-07 13:52 ` Zulkifli, Muhammad Husaini @ 2020-10-07 13:58 ` Michal Simek -1 siblings, 0 replies; 41+ messages in thread From: Michal Simek @ 2020-10-07 13:58 UTC (permalink / raw) To: Zulkifli, Muhammad Husaini, Michal Simek, Hunter, Adrian, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, Wan Mohamad, Wan Ahmad Zainie, arnd Hi, On 07. 10. 20 15:52, Zulkifli, Muhammad Husaini wrote: > Hi, > >> -----Original Message----- >> From: Michal Simek <michal.simek@xilinx.com> >> Sent: Wednesday, October 7, 2020 9:37 PM >> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >> Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian >> <adrian.hunter@intel.com>; sudeep.holla@arm.com; ulf.hansson@linaro.org; >> linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- >> kernel@vger.kernel.org >> Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >> Wan Mohamad, Wan Ahmad Zainie >> <wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >> Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >> Firmware Service call >> >> Hi, >> >> On 07. 10. 20 15:21, Zulkifli, Muhammad Husaini wrote: >>> Hi Michal, >>> >>> Thanks for the feedback. I replied inline >>> >>>> -----Original Message----- >>>> From: Michal Simek <michal.simek@xilinx.com> >>>> Sent: Wednesday, October 7, 2020 4:20 PM >>>> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >>>> Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >>>> sudeep.holla@arm.com; ulf.hansson@linaro.org; >>>> linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >>>> linux-kernel@vger.kernel.org >>>> Cc: Raja Subramanian, Lakshmi Bai >>>> <lakshmi.bai.raja.subramanian@intel.com>; >>>> Wan Mohamad, Wan Ahmad Zainie >>>> <wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >>>> Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm >>>> Trusted Firmware Service call >>>> >>>> Hi, >>>> >>>> 1. Keem Bay: in subject is wrong. Tools are working with it and you >>>> should just use keembay: instead. >>> Are you saying like this ? >>> Keem Bay: Add support for Arm Trusted Firmware Service call >> >> like this: >> firmware: keembay: Add support for Arm Trusted Firmware Service call >> >>> >>>> >>>> 2. This should come first before actual change to keep the tree bisectable. >>> Noted. Done the changes >>>> >>>> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >>>>> From: Muhammad Husaini Zulkifli >>>>> <muhammad.husaini.zulkifli@intel.com> >>>>> >>>>> Add header file to handle API function for device driver to >>>>> communicate with Arm Trusted Firmware. >>>>> >>>>> Signed-off-by: Muhammad Husaini Zulkifli >>>>> <muhammad.husaini.zulkifli@intel.com> >>>>> --- >>>>> .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ >>>>> 1 file changed, 46 insertions(+) >>>>> create mode 100644 include/linux/firmware/intel/keembay_firmware.h >>>>> >>>>> diff --git a/include/linux/firmware/intel/keembay_firmware.h >>>>> b/include/linux/firmware/intel/keembay_firmware.h >>>>> new file mode 100644 >>>>> index 000000000000..9adb8c87b788 >>>>> --- /dev/null >>>>> +++ b/include/linux/firmware/intel/keembay_firmware.h >>>>> @@ -0,0 +1,46 @@ >>>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>>> +/* >>>>> + * Intel Keembay SOC Firmware API Layer >>>>> + * >>>>> + * Copyright (C) 2020-2021, Intel Corporation >>>>> + * >>>>> + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >>>>> + */ >>>>> + >>>>> +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >>>>> +#define __FIRMWARE_KEEMBAY_SMC_H__ >>>>> + >>>>> +#include <linux/arm-smccc.h> >>>>> + >>>>> +/** >>>> >>>> This is not a kernel doc comment. Just use /* >>>> >>>>> + * This file defines API function that can be called by device >>>>> + driver in order to >>>>> + * communicate with Arm Trusted Firmware. >>>>> + */ >>>>> + >>>>> +/* Setting for Keem Bay IO Pad Line Voltage Selection */ >>>>> +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 >>>> >>>> Sudeep: Don't we have any macros for composing these IDs? >>>> nit: IMHO composing these IDs from macros would make more sense to me. >>>> >>>> >>>>> +#define KEEMBAY_SET_1V8_VOLT 0x01 >>>> >>>> 0x01 is just 1 >>> Noted. Done the changes >>>> >>>>> +#define KEEMBAY_SET_3V3_VOLT 0x00 >>>> >>>> 0x00 is just 0 >>> Noted. Done the changes >>>> >>>>> + >>>>> +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) >>>>> +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) { >>>>> + struct arm_smccc_res res; >>>>> + >>>>> + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); >>>>> + >>>>> + return res.a0; >>>> >>>> I am not big fan of this error propagation in case of failure. >>>> >>>> If smc fails you get via res.a0 SMCCC_RET_NOT_SUPPORTED which is >>>> defined as >>>> -1 which is based on errno-base.h defined as EPERM. >>>> >>>> That driver which Sudeep pointed you to is using EINVAL instead. >>>> >>>> It means I would add a code to check it. >>> >>> Yeah I changed to below line of codes. Is this Ok? Tested working. >>> int keembay_sd_voltage_selection(int volt) >> >> static inline here shouldn't hurt. > due to func() prototype " int keembay_sd_voltage_selection(int volt);" to solve warning issues by robot , I cannot set static inline here. > Will observed below error: > > error: static declaration of ‘keembay_sd_voltage_selection’ follows non-static declaration > static inline int keembay_sd_voltage_selection(int volt). Will take a look at when you send new version. Thanks, Michal ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call @ 2020-10-07 13:58 ` Michal Simek 0 siblings, 0 replies; 41+ messages in thread From: Michal Simek @ 2020-10-07 13:58 UTC (permalink / raw) To: Zulkifli, Muhammad Husaini, Michal Simek, Hunter, Adrian, sudeep.holla, ulf.hansson, linux-mmc, linux-arm-kernel, linux-kernel Cc: Raja Subramanian, Lakshmi Bai, arnd, Wan Mohamad, Wan Ahmad Zainie Hi, On 07. 10. 20 15:52, Zulkifli, Muhammad Husaini wrote: > Hi, > >> -----Original Message----- >> From: Michal Simek <michal.simek@xilinx.com> >> Sent: Wednesday, October 7, 2020 9:37 PM >> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >> Michal Simek <michal.simek@xilinx.com>; Hunter, Adrian >> <adrian.hunter@intel.com>; sudeep.holla@arm.com; ulf.hansson@linaro.org; >> linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- >> kernel@vger.kernel.org >> Cc: Raja Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@intel.com>; >> Wan Mohamad, Wan Ahmad Zainie >> <wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >> Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted >> Firmware Service call >> >> Hi, >> >> On 07. 10. 20 15:21, Zulkifli, Muhammad Husaini wrote: >>> Hi Michal, >>> >>> Thanks for the feedback. I replied inline >>> >>>> -----Original Message----- >>>> From: Michal Simek <michal.simek@xilinx.com> >>>> Sent: Wednesday, October 7, 2020 4:20 PM >>>> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; >>>> Hunter, Adrian <adrian.hunter@intel.com>; michal.simek@xilinx.com; >>>> sudeep.holla@arm.com; ulf.hansson@linaro.org; >>>> linux-mmc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >>>> linux-kernel@vger.kernel.org >>>> Cc: Raja Subramanian, Lakshmi Bai >>>> <lakshmi.bai.raja.subramanian@intel.com>; >>>> Wan Mohamad, Wan Ahmad Zainie >>>> <wan.ahmad.zainie.wan.mohamad@intel.com>; arnd@arndb.de >>>> Subject: Re: [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm >>>> Trusted Firmware Service call >>>> >>>> Hi, >>>> >>>> 1. Keem Bay: in subject is wrong. Tools are working with it and you >>>> should just use keembay: instead. >>> Are you saying like this ? >>> Keem Bay: Add support for Arm Trusted Firmware Service call >> >> like this: >> firmware: keembay: Add support for Arm Trusted Firmware Service call >> >>> >>>> >>>> 2. This should come first before actual change to keep the tree bisectable. >>> Noted. Done the changes >>>> >>>> On 06. 10. 20 17:55, muhammad.husaini.zulkifli@intel.com wrote: >>>>> From: Muhammad Husaini Zulkifli >>>>> <muhammad.husaini.zulkifli@intel.com> >>>>> >>>>> Add header file to handle API function for device driver to >>>>> communicate with Arm Trusted Firmware. >>>>> >>>>> Signed-off-by: Muhammad Husaini Zulkifli >>>>> <muhammad.husaini.zulkifli@intel.com> >>>>> --- >>>>> .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++ >>>>> 1 file changed, 46 insertions(+) >>>>> create mode 100644 include/linux/firmware/intel/keembay_firmware.h >>>>> >>>>> diff --git a/include/linux/firmware/intel/keembay_firmware.h >>>>> b/include/linux/firmware/intel/keembay_firmware.h >>>>> new file mode 100644 >>>>> index 000000000000..9adb8c87b788 >>>>> --- /dev/null >>>>> +++ b/include/linux/firmware/intel/keembay_firmware.h >>>>> @@ -0,0 +1,46 @@ >>>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>>> +/* >>>>> + * Intel Keembay SOC Firmware API Layer >>>>> + * >>>>> + * Copyright (C) 2020-2021, Intel Corporation >>>>> + * >>>>> + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@intel.com> >>>>> + */ >>>>> + >>>>> +#ifndef __FIRMWARE_KEEMBAY_SMC_H__ >>>>> +#define __FIRMWARE_KEEMBAY_SMC_H__ >>>>> + >>>>> +#include <linux/arm-smccc.h> >>>>> + >>>>> +/** >>>> >>>> This is not a kernel doc comment. Just use /* >>>> >>>>> + * This file defines API function that can be called by device >>>>> + driver in order to >>>>> + * communicate with Arm Trusted Firmware. >>>>> + */ >>>>> + >>>>> +/* Setting for Keem Bay IO Pad Line Voltage Selection */ >>>>> +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26 >>>> >>>> Sudeep: Don't we have any macros for composing these IDs? >>>> nit: IMHO composing these IDs from macros would make more sense to me. >>>> >>>> >>>>> +#define KEEMBAY_SET_1V8_VOLT 0x01 >>>> >>>> 0x01 is just 1 >>> Noted. Done the changes >>>> >>>>> +#define KEEMBAY_SET_3V3_VOLT 0x00 >>>> >>>> 0x00 is just 0 >>> Noted. Done the changes >>>> >>>>> + >>>>> +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) >>>>> +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1) { >>>>> + struct arm_smccc_res res; >>>>> + >>>>> + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res); >>>>> + >>>>> + return res.a0; >>>> >>>> I am not big fan of this error propagation in case of failure. >>>> >>>> If smc fails you get via res.a0 SMCCC_RET_NOT_SUPPORTED which is >>>> defined as >>>> -1 which is based on errno-base.h defined as EPERM. >>>> >>>> That driver which Sudeep pointed you to is using EINVAL instead. >>>> >>>> It means I would add a code to check it. >>> >>> Yeah I changed to below line of codes. Is this Ok? Tested working. >>> int keembay_sd_voltage_selection(int volt) >> >> static inline here shouldn't hurt. > due to func() prototype " int keembay_sd_voltage_selection(int volt);" to solve warning issues by robot , I cannot set static inline here. > Will observed below error: > > error: static declaration of ‘keembay_sd_voltage_selection’ follows non-static declaration > static inline int keembay_sd_voltage_selection(int volt). Will take a look at when you send new version. Thanks, Michal _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2020-10-07 16:10 UTC | newest] Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-06 15:55 [PATCH v3 0/2] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC muhammad.husaini.zulkifli 2020-10-06 15:55 ` muhammad.husaini.zulkifli 2020-10-06 15:55 ` [PATCH v3 1/2] " muhammad.husaini.zulkifli 2020-10-06 15:55 ` muhammad.husaini.zulkifli 2020-10-07 8:33 ` Michal Simek 2020-10-07 8:33 ` Michal Simek 2020-10-07 8:55 ` Andy Shevchenko 2020-10-07 8:55 ` Andy Shevchenko 2020-10-07 9:10 ` Michal Simek 2020-10-07 9:10 ` Michal Simek 2020-10-07 12:08 ` Andy Shevchenko 2020-10-07 12:08 ` Andy Shevchenko 2020-10-07 13:28 ` Zulkifli, Muhammad Husaini 2020-10-07 13:28 ` Zulkifli, Muhammad Husaini 2020-10-07 14:54 ` Andy Shevchenko 2020-10-07 14:54 ` Andy Shevchenko 2020-10-07 16:08 ` Zulkifli, Muhammad Husaini 2020-10-07 16:08 ` Zulkifli, Muhammad Husaini 2020-10-07 13:25 ` Zulkifli, Muhammad Husaini 2020-10-07 13:25 ` Zulkifli, Muhammad Husaini 2020-10-06 15:55 ` [PATCH v3 2/2] firmware: Keem Bay: Add support for Arm Trusted Firmware Service call muhammad.husaini.zulkifli 2020-10-06 15:55 ` muhammad.husaini.zulkifli 2020-10-06 20:37 ` kernel test robot 2020-10-06 20:37 ` kernel test robot 2020-10-06 20:37 ` kernel test robot 2020-10-07 3:41 ` Zulkifli, Muhammad Husaini 2020-10-07 3:41 ` Zulkifli, Muhammad Husaini 2020-10-07 8:20 ` Michal Simek 2020-10-07 8:20 ` Michal Simek 2020-10-07 10:10 ` Sudeep Holla 2020-10-07 10:10 ` Sudeep Holla 2020-10-07 13:29 ` Zulkifli, Muhammad Husaini 2020-10-07 13:29 ` Zulkifli, Muhammad Husaini 2020-10-07 13:21 ` Zulkifli, Muhammad Husaini 2020-10-07 13:21 ` Zulkifli, Muhammad Husaini 2020-10-07 13:37 ` Michal Simek 2020-10-07 13:37 ` Michal Simek 2020-10-07 13:52 ` Zulkifli, Muhammad Husaini 2020-10-07 13:52 ` Zulkifli, Muhammad Husaini 2020-10-07 13:58 ` Michal Simek 2020-10-07 13:58 ` Michal Simek
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.