From: Subhash Jadavani <[email protected]>
UFS host supplies the reference clock to UFS device and UFS device
specification allows host to provide one of the 4 frequencies (19.2 MHz,
26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
device reference clock frequency setting in the device based on what
frequency it is supplying to UFS device.
Signed-off-by: Subhash Jadavani <[email protected]>
Signed-off-by: Can Guo <[email protected]>
Signed-off-by: Sayali Lokhande <[email protected]>
---
.../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 7 +++
drivers/scsi/ufs/ufs.h | 9 ++++
drivers/scsi/ufs/ufshcd-pltfrm.c | 24 ++++++++++
drivers/scsi/ufs/ufshcd.c | 52 ++++++++++++++++++++++
drivers/scsi/ufs/ufshcd.h | 1 +
5 files changed, 93 insertions(+)
diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
index c39dfef..4522434 100644
--- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
+++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
@@ -41,6 +41,12 @@ Optional properties:
-lanes-per-direction : number of lanes available per direction - either 1 or 2.
Note that it is assume same number of lanes is used both
directions at once. If not specified, default is 2 lanes per direction.
+- dev-ref-clk-freq : Specify the device reference clock frequency, must be one of the following:
+ 0: 19.2 MHz
+ 1: 26 MHz
+ 2: 38.4 MHz
+ 3: 52 MHz
+ Defaults to 26 MHz if not specified.
Note: If above properties are not defined it can be assumed that the supply
regulators or clocks are always on.
@@ -66,4 +72,5 @@ Example:
freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
phys = <&ufsphy1>;
phy-names = "ufsphy";
+ dev-ref-clk-freq = <0>; /* reference clock freq: 19.2 MHz */
};
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 14e5bf7..e15deb0 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -378,6 +378,15 @@ enum query_opcode {
UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
};
+/* bRefClkFreq attribute values */
+enum ref_clk_freq {
+ REF_CLK_FREQ_19_2_MHZ = 0x0,
+ REF_CLK_FREQ_26_MHZ = 0x1,
+ REF_CLK_FREQ_38_4_MHZ = 0x2,
+ REF_CLK_FREQ_52_MHZ = 0x3,
+ REF_CLK_FREQ_MAX = REF_CLK_FREQ_52_MHZ,
+};
+
/* Query response result code */
enum {
QUERY_RESULT_SUCCESS = 0x00,
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index e82bde0..6c877f3 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -221,6 +221,28 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
return err;
}
+static void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba)
+{
+ struct device *dev = hba->dev;
+ struct device_node *np = dev->of_node;
+ int ret;
+
+ if (!np)
+ return;
+
+ ret = of_property_read_u32(np, "dev-ref-clk-freq",
+ &hba->dev_ref_clk_freq);
+ if (ret ||
+ (hba->dev_ref_clk_freq < 0) ||
+ (hba->dev_ref_clk_freq > REF_CLK_FREQ_52_MHZ)) {
+ dev_err(hba->dev,
+ "%s: invalid ref_clk setting = %d, set to default\n",
+ __func__, hba->dev_ref_clk_freq);
+ /* default setting */
+ hba->dev_ref_clk_freq = REF_CLK_FREQ_26_MHZ;
+ }
+}
+
#ifdef CONFIG_PM
/**
* ufshcd_pltfrm_suspend - suspend power management function
@@ -343,6 +365,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
+ ufshcd_parse_dev_ref_clk_freq(hba);
+
ufshcd_init_lanes_per_dir(hba);
err = ufshcd_init(hba, mmio_base, irq);
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c5b1bf1..4abc7ae 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6297,6 +6297,53 @@ static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
}
/**
+ * ufshcd_set_dev_ref_clk - set the device bRefClkFreq
+ * @hba: per-adapter instance
+ *
+ * Read the current value of the bRefClkFreq attribute from device and update it
+ * if host is supplying different reference clock frequency than one mentioned
+ * in bRefClkFreq attribute.
+ *
+ * Returns zero on success, non-zero error value on failure.
+ */
+static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
+{
+ int err = 0;
+ int ref_clk = -1;
+ static const char * const ref_clk_freqs[] = {"19.2 MHz", "26 MHz",
+ "38.4 MHz", "52 MHz"};
+
+ err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+ QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0, &ref_clk);
+
+ if (err) {
+ dev_err(hba->dev, "%s: failed reading bRefClkFreq. err = %d\n",
+ __func__, err);
+ goto out;
+ }
+
+ if (ref_clk == hba->dev_ref_clk_freq)
+ goto out; /* nothing to update */
+
+ err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
+ QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0,
+ &hba->dev_ref_clk_freq);
+
+ if (err)
+ dev_err(hba->dev, "%s: bRefClkFreq setting to %s failed\n",
+ __func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
+ /*
+ * It is good to print this out here to debug any later failures
+ * related to gear switch.
+ */
+ dev_dbg(hba->dev, "%s: bRefClkFreq setting to %s succeeded\n",
+ __func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
+
+out:
+ return err;
+}
+
+/**
* ufshcd_probe_hba - probe hba to detect device and initialize
* @hba: per-adapter instance
*
@@ -6361,6 +6408,11 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
"%s: Failed getting max supported power mode\n",
__func__);
} else {
+ /*
+ * Set the right value to bRefClkFreq before attempting to
+ * switch to HS gears.
+ */
+ ufshcd_set_dev_ref_clk(hba);
ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
if (ret) {
dev_err(hba->dev, "%s: Failed setting power mode, err = %d\n",
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 8110dcd..b026ad8 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -548,6 +548,7 @@ struct ufs_hba {
void *priv;
unsigned int irq;
bool is_irq_enabled;
+ u32 dev_ref_clk_freq;
/* Interrupt aggregation support is broken */
#define UFSHCD_QUIRK_BROKEN_INTR_AGGR 0x1
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
On 08/06/18 14:06, Sayali Lokhande wrote:
> From: Subhash Jadavani <[email protected]>
>
> UFS host supplies the reference clock to UFS device and UFS device
> specification allows host to provide one of the 4 frequencies (19.2 MHz,
> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
> device reference clock frequency setting in the device based on what
> frequency it is supplying to UFS device.
>
> Signed-off-by: Subhash Jadavani <[email protected]>
> Signed-off-by: Can Guo <[email protected]>
> Signed-off-by: Sayali Lokhande <[email protected]>
> ---
> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 7 +++
> drivers/scsi/ufs/ufs.h | 9 ++++
> drivers/scsi/ufs/ufshcd-pltfrm.c | 24 ++++++++++
> drivers/scsi/ufs/ufshcd.c | 52 ++++++++++++++++++++++
> drivers/scsi/ufs/ufshcd.h | 1 +
> 5 files changed, 93 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index c39dfef..4522434 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -41,6 +41,12 @@ Optional properties:
> -lanes-per-direction : number of lanes available per direction - either 1 or 2.
> Note that it is assume same number of lanes is used both
> directions at once. If not specified, default is 2 lanes per direction.
> +- dev-ref-clk-freq : Specify the device reference clock frequency, must be one of the following:
> + 0: 19.2 MHz
> + 1: 26 MHz
> + 2: 38.4 MHz
> + 3: 52 MHz
> + Defaults to 26 MHz if not specified.
>
> Note: If above properties are not defined it can be assumed that the supply
> regulators or clocks are always on.
> @@ -66,4 +72,5 @@ Example:
> freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
> phys = <&ufsphy1>;
> phy-names = "ufsphy";
> + dev-ref-clk-freq = <0>; /* reference clock freq: 19.2 MHz */
> };
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index 14e5bf7..e15deb0 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -378,6 +378,15 @@ enum query_opcode {
> UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8,
> };
>
> +/* bRefClkFreq attribute values */
> +enum ref_clk_freq {
> + REF_CLK_FREQ_19_2_MHZ = 0x0,
> + REF_CLK_FREQ_26_MHZ = 0x1,
> + REF_CLK_FREQ_38_4_MHZ = 0x2,
> + REF_CLK_FREQ_52_MHZ = 0x3,
> + REF_CLK_FREQ_MAX = REF_CLK_FREQ_52_MHZ,
> +};
> +
> /* Query response result code */
> enum {
> QUERY_RESULT_SUCCESS = 0x00,
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index e82bde0..6c877f3 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -221,6 +221,28 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
> return err;
> }
>
> +static void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba)
> +{
> + struct device *dev = hba->dev;
> + struct device_node *np = dev->of_node;
> + int ret;
> +
> + if (!np)
> + return;
> +
> + ret = of_property_read_u32(np, "dev-ref-clk-freq",
> + &hba->dev_ref_clk_freq);
This setting is useful for any UFSHC driver. Please move it to ufshcd.c and
use device_property_read_u32().
> + if (ret ||
> + (hba->dev_ref_clk_freq < 0) ||
u32 cannot be < 0
> + (hba->dev_ref_clk_freq > REF_CLK_FREQ_52_MHZ)) {
> + dev_err(hba->dev,
> + "%s: invalid ref_clk setting = %d, set to default\n",
> + __func__, hba->dev_ref_clk_freq);
> + /* default setting */
> + hba->dev_ref_clk_freq = REF_CLK_FREQ_26_MHZ;
No, the default is to leave the value unchanged.
> + }
> +}
> +
> #ifdef CONFIG_PM
> /**
> * ufshcd_pltfrm_suspend - suspend power management function
> @@ -343,6 +365,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
> pm_runtime_set_active(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
>
> + ufshcd_parse_dev_ref_clk_freq(hba);
> +
> ufshcd_init_lanes_per_dir(hba);
>
> err = ufshcd_init(hba, mmio_base, irq);
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index c5b1bf1..4abc7ae 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6297,6 +6297,53 @@ static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
> }
>
> /**
> + * ufshcd_set_dev_ref_clk - set the device bRefClkFreq
> + * @hba: per-adapter instance
> + *
> + * Read the current value of the bRefClkFreq attribute from device and update it
> + * if host is supplying different reference clock frequency than one mentioned
> + * in bRefClkFreq attribute.
> + *
> + * Returns zero on success, non-zero error value on failure.
> + */
> +static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
> +{
> + int err = 0;
> + int ref_clk = -1;
> + static const char * const ref_clk_freqs[] = {"19.2 MHz", "26 MHz",
> + "38.4 MHz", "52 MHz"};
> +
> + err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> + QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0, &ref_clk);
> +
> + if (err) {
> + dev_err(hba->dev, "%s: failed reading bRefClkFreq. err = %d\n",
> + __func__, err);
> + goto out;
> + }
> +
> + if (ref_clk == hba->dev_ref_clk_freq)
> + goto out; /* nothing to update */
> +
> + err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> + QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0,
> + &hba->dev_ref_clk_freq);
> +
> + if (err)
> + dev_err(hba->dev, "%s: bRefClkFreq setting to %s failed\n",
> + __func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
> + /*
> + * It is good to print this out here to debug any later failures
> + * related to gear switch.
> + */
> + dev_dbg(hba->dev, "%s: bRefClkFreq setting to %s succeeded\n",
> + __func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
> +
> +out:
> + return err;
> +}
> +
> +/**
> * ufshcd_probe_hba - probe hba to detect device and initialize
> * @hba: per-adapter instance
> *
> @@ -6361,6 +6408,11 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
> "%s: Failed getting max supported power mode\n",
> __func__);
> } else {
> + /*
> + * Set the right value to bRefClkFreq before attempting to
> + * switch to HS gears.
> + */
> + ufshcd_set_dev_ref_clk(hba);
> ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
> if (ret) {
> dev_err(hba->dev, "%s: Failed setting power mode, err = %d\n",
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 8110dcd..b026ad8 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -548,6 +548,7 @@ struct ufs_hba {
> void *priv;
> unsigned int irq;
> bool is_irq_enabled;
> + u32 dev_ref_clk_freq;
>
> /* Interrupt aggregation support is broken */
> #define UFSHCD_QUIRK_BROKEN_INTR_AGGR 0x1
>
On Fri, Jun 08, 2018 at 04:36:28PM +0530, Sayali Lokhande wrote:
> From: Subhash Jadavani <[email protected]>
>
> UFS host supplies the reference clock to UFS device and UFS device
> specification allows host to provide one of the 4 frequencies (19.2 MHz,
> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
> device reference clock frequency setting in the device based on what
> frequency it is supplying to UFS device.
>
> Signed-off-by: Subhash Jadavani <[email protected]>
> Signed-off-by: Can Guo <[email protected]>
> Signed-off-by: Sayali Lokhande <[email protected]>
> ---
> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 7 +++
> drivers/scsi/ufs/ufs.h | 9 ++++
> drivers/scsi/ufs/ufshcd-pltfrm.c | 24 ++++++++++
> drivers/scsi/ufs/ufshcd.c | 52 ++++++++++++++++++++++
> drivers/scsi/ufs/ufshcd.h | 1 +
> 5 files changed, 93 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index c39dfef..4522434 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -41,6 +41,12 @@ Optional properties:
> -lanes-per-direction : number of lanes available per direction - either 1 or 2.
> Note that it is assume same number of lanes is used both
> directions at once. If not specified, default is 2 lanes per direction.
> +- dev-ref-clk-freq : Specify the device reference clock frequency, must be one of the following:
> + 0: 19.2 MHz
> + 1: 26 MHz
> + 2: 38.4 MHz
> + 3: 52 MHz
> + Defaults to 26 MHz if not specified.
I must have misunderstood your last response. I thought you could handle
things without DT. If not, my question remains.
Rob
Comment inline.
Thanks,
Sayali
-----Original Message-----
From: Rob Herring [mailto:[email protected]]
Sent: Wednesday, June 13, 2018 12:57 AM
To: Sayali Lokhande <[email protected]>
Cc: [email protected]; [email protected];
[email protected]; [email protected]; [email protected];
[email protected]; [email protected];
[email protected]; [email protected]; [email protected];
Mark Rutland <[email protected]>; Mathieu Malaterre <[email protected]>;
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
<[email protected]>; open list <[email protected]>
Subject: Re: [PATCH V2 1/3] scsi: ufs: set the device reference clock
setting
On Fri, Jun 08, 2018 at 04:36:28PM +0530, Sayali Lokhande wrote:
> From: Subhash Jadavani <[email protected]>
>
> UFS host supplies the reference clock to UFS device and UFS device
> specification allows host to provide one of the 4 frequencies (19.2
> MHz,
> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
> device reference clock frequency setting in the device based on what
> frequency it is supplying to UFS device.
>
> Signed-off-by: Subhash Jadavani <[email protected]>
> Signed-off-by: Can Guo <[email protected]>
> Signed-off-by: Sayali Lokhande <[email protected]>
> ---
> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 7 +++
> drivers/scsi/ufs/ufs.h | 9 ++++
> drivers/scsi/ufs/ufshcd-pltfrm.c | 24 ++++++++++
> drivers/scsi/ufs/ufshcd.c | 52
++++++++++++++++++++++
> drivers/scsi/ufs/ufshcd.h | 1 +
> 5 files changed, 93 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index c39dfef..4522434 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -41,6 +41,12 @@ Optional properties:
> -lanes-per-direction : number of lanes available per direction - either 1
or 2.
> Note that it is assume same number of lanes is
used both
> directions at once. If not specified, default is 2
lanes per direction.
> +- dev-ref-clk-freq : Specify the device reference clock frequency, must
be one of the following:
> + 0: 19.2 MHz
> + 1: 26 MHz
> + 2: 38.4 MHz
> + 3: 52 MHz
> + Defaults to 26 MHz if not specified.
I must have misunderstood your last response. I thought you could handle
things without DT. If not, my question remains.
[Sayali]: Ref clk frequency setting could vary from
platfrom-to-platform(vendor specific). Hence we need to pass it via DT.
Currently in DT we do not set/mention any ref clk frequency
parameter. Hence I have added one new DT entry to configure
required ref clk freq.
Rob
On Thu, 2018-06-14 at 17:03 +0530, sayali wrote:
> Comment inline.
Hello Sayali,
Please don't use inline replies. That makes it very hard to follow the
conversation. BTW, in the headers of your e-mail I found the following:
"X-Mailer: Microsoft Outlook 16.0". Please use another e-mail client that is
better suited for collaboration on open source mailing lists. If outgoing
e-mails have to pass through an Exchange server, the following e-mail clients
support Exchange servers and are better suited for open source collaboration:
* Evolution (Linux only).
* Thunderbird + ExQuilla plugin.
* If IMAP has been enabled on the Exchange server that you are using then
that means that you can choose from the many open source e-mail clients
that support IMAP.
Thanks,
Bart.
On Thu, Jun 14, 2018 at 5:33 AM, sayali <[email protected]> wrote:
> Comment inline.
>
> Thanks,
> Sayali
> -----Original Message-----
> From: Rob Herring [mailto:[email protected]]
> Sent: Wednesday, June 13, 2018 12:57 AM
> To: Sayali Lokhande <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Mark Rutland <[email protected]>; Mathieu Malaterre <[email protected]>;
> open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <[email protected]>; open list <[email protected]>
> Subject: Re: [PATCH V2 1/3] scsi: ufs: set the device reference clock
> setting
>
> On Fri, Jun 08, 2018 at 04:36:28PM +0530, Sayali Lokhande wrote:
>> From: Subhash Jadavani <[email protected]>
>>
>> UFS host supplies the reference clock to UFS device and UFS device
>> specification allows host to provide one of the 4 frequencies (19.2
>> MHz,
>> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
>> device reference clock frequency setting in the device based on what
>> frequency it is supplying to UFS device.
>>
>> Signed-off-by: Subhash Jadavani <[email protected]>
>> Signed-off-by: Can Guo <[email protected]>
>> Signed-off-by: Sayali Lokhande <[email protected]>
>> ---
>> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 7 +++
>> drivers/scsi/ufs/ufs.h | 9 ++++
>> drivers/scsi/ufs/ufshcd-pltfrm.c | 24 ++++++++++
>> drivers/scsi/ufs/ufshcd.c | 52
> ++++++++++++++++++++++
>> drivers/scsi/ufs/ufshcd.h | 1 +
>> 5 files changed, 93 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> index c39dfef..4522434 100644
>> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> @@ -41,6 +41,12 @@ Optional properties:
>> -lanes-per-direction : number of lanes available per direction - either 1
> or 2.
>> Note that it is assume same number of lanes is
> used both
>> directions at once. If not specified, default is 2
> lanes per direction.
>> +- dev-ref-clk-freq : Specify the device reference clock frequency, must
> be one of the following:
>> + 0: 19.2 MHz
>> + 1: 26 MHz
>> + 2: 38.4 MHz
>> + 3: 52 MHz
>> + Defaults to 26 MHz if not specified.
>
> I must have misunderstood your last response. I thought you could handle
> things without DT. If not, my question remains.
> [Sayali]: Ref clk frequency setting could vary from
> platfrom-to-platform(vendor specific). Hence we need to pass it via DT.
> Currently in DT we do not set/mention any ref clk frequency
> parameter. Hence I have added one new DT entry to configure
> required ref clk freq.
The clocks property contains "ref_clk". Is that not the same clock?
Why can't you read what that frequency is? Or you need to be able to
change it to a specific frequency? These all look like typical
oscillator frequencies, so I wouldn't expect you could change them
(other than divide by 2 maybe).
Rob
Actually, it makes it easier to follow the conversation.
On Thu, 14 Jun 2018 13:35:08 -0000, Bart Van Assche said:
> On Thu, 2018-06-14 at 17:03 +0530, sayali wrote:
> > Comment inline.
>
> Hello Sayali,
>
> Please don't use inline replies. That makes it very hard to follow the
> conversation.
Actually, it makes it easier to follow the conversation.
> * If IMAP has been enabled on the Exchange server that you are using then
> that means that you can choose from the many open source e-mail clients
> that support IMAP.
Which of the two is easier to follow?
On Fri, 2018-06-15 at 16:37 -0400, [email protected] wrote:
> Actually, it makes it easier to follow the conversation.
In case I wasn't clear enough: I was referring to replying inline
*without* quoting the original message. That makes a conversation
*really* hard to follow.
Bart.
On Fri, 15 Jun 2018 20:42:41 -0000, Bart Van Assche said:
> In case I wasn't clear enough: I was referring to replying inline
> *without* quoting the original message. That makes a conversation
> *really* hard to follow.
So if there's no quoted message, what is the inline reply inlined into? :)
On Fri, 2018-06-15 at 16:53 -0400, [email protected] wrote:
> On Fri, 15 Jun 2018 20:42:41 -0000, Bart Van Assche said:
> > In case I wasn't clear enough: I was referring to replying inline
> > *without* quoting the original message. That makes a conversation
> > *really* hard to follow.
>
> So if there's no quoted message, what is the inline reply inlined into? :)
I was referring to quoting without indenting the quote with "> ". That
should already have been clear if you had a look at the message I complained
about.
Bart.
Hi Rob,
Please check my comment inline.
Thanks,
Sayali
-----Original Message-----
From: Rob Herring [mailto:[email protected]]
Sent: Thursday, June 14, 2018 7:59 PM
To: sayali <[email protected]>
Cc: Subhash Jadavani <[email protected]>; Can Guo <[email protected]>; Vivek Gautam <[email protected]>; Rajendra Nayak <[email protected]>; Vinayak Holikatti <[email protected]>; James E.J. Bottomley <[email protected]>; Martin K. Petersen <[email protected]>; [email protected]; Evan Green <[email protected]>; [email protected]; Mark Rutland <[email protected]>; Mathieu Malaterre <[email protected]>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <[email protected]>; open list <[email protected]>
Subject: Re: [PATCH V2 1/3] scsi: ufs: set the device reference clock setting
On Thu, Jun 14, 2018 at 5:33 AM, sayali <[email protected]> wrote:
> Comment inline.
>
> Thanks,
> Sayali
> -----Original Message-----
> From: Rob Herring [mailto:[email protected]]
> Sent: Wednesday, June 13, 2018 12:57 AM
> To: Sayali Lokhande <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; Mark Rutland
> <[email protected]>; Mathieu Malaterre <[email protected]>; open
> list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <[email protected]>; open list <[email protected]>
> Subject: Re: [PATCH V2 1/3] scsi: ufs: set the device reference clock
> setting
>
> On Fri, Jun 08, 2018 at 04:36:28PM +0530, Sayali Lokhande wrote:
>> From: Subhash Jadavani <[email protected]>
>>
>> UFS host supplies the reference clock to UFS device and UFS device
>> specification allows host to provide one of the 4 frequencies (19.2
>> MHz,
>> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
>> device reference clock frequency setting in the device based on what
>> frequency it is supplying to UFS device.
>>
>> Signed-off-by: Subhash Jadavani <[email protected]>
>> Signed-off-by: Can Guo <[email protected]>
>> Signed-off-by: Sayali Lokhande <[email protected]>
>> ---
>> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 7 +++
>> drivers/scsi/ufs/ufs.h | 9 ++++
>> drivers/scsi/ufs/ufshcd-pltfrm.c | 24 ++++++++++
>> drivers/scsi/ufs/ufshcd.c | 52
> ++++++++++++++++++++++
>> drivers/scsi/ufs/ufshcd.h | 1 +
>> 5 files changed, 93 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> index c39dfef..4522434 100644
>> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> @@ -41,6 +41,12 @@ Optional properties:
>> -lanes-per-direction : number of lanes available per direction -
>> either 1
> or 2.
>> Note that it is assume same number of lanes
>> is
> used both
>> directions at once. If not specified, default
>> is 2
> lanes per direction.
>> +- dev-ref-clk-freq : Specify the device reference clock frequency, must
> be one of the following:
>> + 0: 19.2 MHz
>> + 1: 26 MHz
>> + 2: 38.4 MHz
>> + 3: 52 MHz
>> + Defaults to 26 MHz if not specified.
>
> I must have misunderstood your last response. I thought you could
> handle things without DT. If not, my question remains.
> [Sayali]: Ref clk frequency setting could vary from
> platfrom-to-platform(vendor specific). Hence we need to pass it via DT.
> Currently in DT we do not set/mention any ref clk frequency
> parameter. Hence I have added one new DT entry to configure
> required ref clk freq.
The clocks property contains "ref_clk". Is that not the same clock?
Why can't you read what that frequency is? Or you need to be able to change it to a specific frequency? These all look like typical oscillator frequencies, so I wouldn't expect you could change them (other than divide by 2 maybe).
[Sayali] : It is the same "ref_clk", but we need to be able to change it to a specific frequency as per requirement. Thus, we need new DT entry to specify/override reference clock frequency as per need.
Rob
On Thu, Jun 21, 2018 at 2:52 AM sayali <[email protected]> wrote:
>
> Hi Rob,
>
> Please check my comment inline.
As mentioned in the back and forth comments previously in this thread,
please fix your email client (hint: you can't use Outlook) and
properly quote your replies (i.e. the leading ">")
> Thanks,
> Sayali
> -----Original Message-----
> From: Rob Herring [mailto:[email protected]]
> Sent: Thursday, June 14, 2018 7:59 PM
> To: sayali <[email protected]>
> Cc: Subhash Jadavani <[email protected]>; Can Guo <[email protected]>; Vivek Gautam <[email protected]>; Rajendra Nayak <[email protected]>; Vinayak Holikatti <[email protected]>; James E.J. Bottomley <[email protected]>; Martin K. Petersen <[email protected]>; [email protected]; Evan Green <[email protected]>; [email protected]; Mark Rutland <[email protected]>; Mathieu Malaterre <[email protected]>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <[email protected]>; open list <[email protected]>
> Subject: Re: [PATCH V2 1/3] scsi: ufs: set the device reference clock setting
>
> On Thu, Jun 14, 2018 at 5:33 AM, sayali <[email protected]> wrote:
> > Comment inline.
> >
> > Thanks,
> > Sayali
> > -----Original Message-----
> > From: Rob Herring [mailto:[email protected]]
> > Sent: Wednesday, June 13, 2018 12:57 AM
> > To: Sayali Lokhande <[email protected]>
> > Cc: [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; Mark Rutland
> > <[email protected]>; Mathieu Malaterre <[email protected]>; open
> > list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> > <[email protected]>; open list <[email protected]>
> > Subject: Re: [PATCH V2 1/3] scsi: ufs: set the device reference clock
> > setting
> >
> > On Fri, Jun 08, 2018 at 04:36:28PM +0530, Sayali Lokhande wrote:
> >> From: Subhash Jadavani <[email protected]>
> >>
> >> UFS host supplies the reference clock to UFS device and UFS device
> >> specification allows host to provide one of the 4 frequencies (19.2
> >> MHz,
> >> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
> >> device reference clock frequency setting in the device based on what
> >> frequency it is supplying to UFS device.
> >>
> >> Signed-off-by: Subhash Jadavani <[email protected]>
> >> Signed-off-by: Can Guo <[email protected]>
> >> Signed-off-by: Sayali Lokhande <[email protected]>
> >> ---
> >> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 7 +++
> >> drivers/scsi/ufs/ufs.h | 9 ++++
> >> drivers/scsi/ufs/ufshcd-pltfrm.c | 24 ++++++++++
> >> drivers/scsi/ufs/ufshcd.c | 52
> > ++++++++++++++++++++++
> >> drivers/scsi/ufs/ufshcd.h | 1 +
> >> 5 files changed, 93 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> >> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> >> index c39dfef..4522434 100644
> >> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> >> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> >> @@ -41,6 +41,12 @@ Optional properties:
> >> -lanes-per-direction : number of lanes available per direction -
> >> either 1
> > or 2.
> >> Note that it is assume same number of lanes
> >> is
> > used both
> >> directions at once. If not specified, default
> >> is 2
> > lanes per direction.
> >> +- dev-ref-clk-freq : Specify the device reference clock frequency, must
> > be one of the following:
> >> + 0: 19.2 MHz
> >> + 1: 26 MHz
> >> + 2: 38.4 MHz
> >> + 3: 52 MHz
> >> + Defaults to 26 MHz if not specified.
> >
> > I must have misunderstood your last response. I thought you could
> > handle things without DT. If not, my question remains.
> > [Sayali]: Ref clk frequency setting could vary from
> > platfrom-to-platform(vendor specific). Hence we need to pass it via DT.
> > Currently in DT we do not set/mention any ref clk frequency
> > parameter. Hence I have added one new DT entry to configure
> > required ref clk freq.
>
> The clocks property contains "ref_clk". Is that not the same clock?
> Why can't you read what that frequency is? Or you need to be able to change it to a specific frequency? These all look like typical oscillator frequencies, so I wouldn't expect you could change them (other than divide by 2 maybe).
> [Sayali] : It is the same "ref_clk", but we need to be able to change it to a specific frequency as per requirement. Thus, we need new DT entry to specify/override reference clock frequency as per need.
That is not what your patch does. It just tells the device what the
frequency is. If you need to get the rate, use "clk_get_rate" on
"ref_clk". If you need to actually set it to a specific frequency,
then we have properties for that already (assigned-clock-rates).
Seems to me that by the time you get to Linux, the bootloader would
have already set this. Otherwise, how do you boot? Seems like you
would want to read the attr and ensure "ref_clk" freq matches.
Rob
On 7/3/2018 11:34 PM, Rob Herring wrote:
> On Thu, Jun 21, 2018 at 2:52 AM sayali <[email protected]> wrote:
>> Hi Rob,
>>
>> Please check my comment inline.
> As mentioned in the back and forth comments previously in this thread,
> please fix your email client (hint: you can't use Outlook) and
> properly quote your replies (i.e. the leading ">")
I have started using Thunderbird now. Hope you can see my replies
correctly this time.
>
>> Thanks,
>> Sayali
>> -----Original Message-----
>> From: Rob Herring [mailto:[email protected]]
>> Sent: Thursday, June 14, 2018 7:59 PM
>> To: sayali <[email protected]>
>> Cc: Subhash Jadavani <[email protected]>; Can Guo <[email protected]>; Vivek Gautam <[email protected]>; Rajendra Nayak <[email protected]>; Vinayak Holikatti <[email protected]>; James E.J. Bottomley <[email protected]>; Martin K. Petersen <[email protected]>; [email protected]; Evan Green <[email protected]>; [email protected]; Mark Rutland <[email protected]>; Mathieu Malaterre <[email protected]>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <[email protected]>; open list <[email protected]>
>> Subject: Re: [PATCH V2 1/3] scsi: ufs: set the device reference clock setting
>>
>> On Thu, Jun 14, 2018 at 5:33 AM, sayali <[email protected]> wrote:
>>> Comment inline.
>>>
>>> Thanks,
>>> Sayali
>>> -----Original Message-----
>>> From: Rob Herring [mailto:[email protected]]
>>> Sent: Wednesday, June 13, 2018 12:57 AM
>>> To: Sayali Lokhande <[email protected]>
>>> Cc: [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected];
>>> [email protected]; [email protected]; Mark Rutland
>>> <[email protected]>; Mathieu Malaterre <[email protected]>; open
>>> list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
>>> <[email protected]>; open list <[email protected]>
>>> Subject: Re: [PATCH V2 1/3] scsi: ufs: set the device reference clock
>>> setting
>>>
>>> On Fri, Jun 08, 2018 at 04:36:28PM +0530, Sayali Lokhande wrote:
>>>> From: Subhash Jadavani <[email protected]>
>>>>
>>>> UFS host supplies the reference clock to UFS device and UFS device
>>>> specification allows host to provide one of the 4 frequencies (19.2
>>>> MHz,
>>>> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
>>>> device reference clock frequency setting in the device based on what
>>>> frequency it is supplying to UFS device.
>>>>
>>>> Signed-off-by: Subhash Jadavani <[email protected]>
>>>> Signed-off-by: Can Guo <[email protected]>
>>>> Signed-off-by: Sayali Lokhande <[email protected]>
>>>> ---
>>>> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 7 +++
>>>> drivers/scsi/ufs/ufs.h | 9 ++++
>>>> drivers/scsi/ufs/ufshcd-pltfrm.c | 24 ++++++++++
>>>> drivers/scsi/ufs/ufshcd.c | 52
>>> ++++++++++++++++++++++
>>>> drivers/scsi/ufs/ufshcd.h | 1 +
>>>> 5 files changed, 93 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>>>> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>>>> index c39dfef..4522434 100644
>>>> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>>>> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>>>> @@ -41,6 +41,12 @@ Optional properties:
>>>> -lanes-per-direction : number of lanes available per direction -
>>>> either 1
>>> or 2.
>>>> Note that it is assume same number of lanes
>>>> is
>>> used both
>>>> directions at once. If not specified, default
>>>> is 2
>>> lanes per direction.
>>>> +- dev-ref-clk-freq : Specify the device reference clock frequency, must
>>> be one of the following:
>>>> + 0: 19.2 MHz
>>>> + 1: 26 MHz
>>>> + 2: 38.4 MHz
>>>> + 3: 52 MHz
>>>> + Defaults to 26 MHz if not specified.
>>> I must have misunderstood your last response. I thought you could
>>> handle things without DT. If not, my question remains.
>>> [Sayali]: Ref clk frequency setting could vary from
>>> platfrom-to-platform(vendor specific). Hence we need to pass it via DT.
>>> Currently in DT we do not set/mention any ref clk frequency
>>> parameter. Hence I have added one new DT entry to configure
>>> required ref clk freq.
>> The clocks property contains "ref_clk". Is that not the same clock?
>> Why can't you read what that frequency is? Or you need to be able to change it to a specific frequency? These all look like typical oscillator frequencies, so I wouldn't expect you could change them (other than divide by 2 maybe).
>> [Sayali] : It is the same "ref_clk", but we need to be able to change it to a specific frequency as per requirement. Thus, we need new DT entry to specify/override reference clock frequency as per need.
> That is not what your patch does. It just tells the device what the
> frequency is. If you need to get the rate, use "clk_get_rate" on
> "ref_clk". If you need to actually set it to a specific frequency,
> then we have properties for that already (assigned-clock-rates).
Yes, we need to set it to a specific frequency. I will use
"assigned-clock-rates" for passing ref_clk frequency in next patch set.
>
> Seems to me that by the time you get to Linux, the bootloader would
> have already set this. Otherwise, how do you boot? Seems like you
> would want to read the attr and ensure "ref_clk" freq matches.
Yes, ref_clk freq will be already set in device via bootloader. In
Kernel, before updating ref_clk freq (if required and passed via DT), we
are reading the current ref_clk set in device and only if it is
different than what has been passed via DT, we will be updating ref_clk
freq, otherwise we just return.
> Rob
Thanks,
Sayali