aemif_calc_rate() check the validity of a new computed timing against a
'max' value given as input. This isn't convenient if we want to check
the CS timing configuration somewhere else in the code.
Wrap the verification of all the chip select's timing configuration into a
single function to ease its exportation in upcoming patches.
Remove the 'max' input from aemif_calc_rate() as it's no longer used.
Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
drivers/memory/ti-aemif.c | 107 +++++++++++++++++---------------------
1 file changed, 49 insertions(+), 58 deletions(-)
diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
index aec6d6464efa..5c1c6f95185f 100644
--- a/drivers/memory/ti-aemif.c
+++ b/drivers/memory/ti-aemif.c
@@ -132,18 +132,48 @@ struct aemif_device {
struct aemif_cs_data cs_data[NUM_CS];
};
+/**
+ * aemif_check_cs_timings - Check the validity of a CS timing configuration.
+ * @timings: timings configuration
+ *
+ * @return: 0 if the timing configuration is valid, negative error number otherwise.
+ */
+static int aemif_check_cs_timings(struct aemif_cs_timings *timings)
+{
+ if (timings->ta > TA_MAX)
+ return -EINVAL;
+
+ if (timings->rhold > RHOLD_MAX)
+ return -EINVAL;
+
+ if (timings->rstrobe > RSTROBE_MAX)
+ return -EINVAL;
+
+ if (timings->rsetup > RSETUP_MAX)
+ return -EINVAL;
+
+ if (timings->whold > WHOLD_MAX)
+ return -EINVAL;
+
+ if (timings->wstrobe > WSTROBE_MAX)
+ return -EINVAL;
+
+ if (timings->wsetup > WSETUP_MAX)
+ return -EINVAL;
+
+ return 0;
+}
+
/**
* aemif_calc_rate - calculate timing data.
* @pdev: platform device to calculate for
* @wanted: The cycle time needed in nanoseconds.
* @clk: The input clock rate in kHz.
- * @max: The maximum divider value that can be programmed.
*
* On success, returns the calculated timing value minus 1 for easy
* programming into AEMIF timing registers, else negative errno.
*/
-static int aemif_calc_rate(struct platform_device *pdev, int wanted,
- unsigned long clk, int max)
+static int aemif_calc_rate(struct platform_device *pdev, int wanted, unsigned long clk)
{
int result;
@@ -156,10 +186,6 @@ static int aemif_calc_rate(struct platform_device *pdev, int wanted,
if (result < 0)
result = 0;
- /* ... But configuring tighter timings is not an option. */
- else if (result > max)
- result = -EINVAL;
-
return result;
}
@@ -249,7 +275,6 @@ static int of_aemif_parse_abus_config(struct platform_device *pdev,
struct aemif_device *aemif = platform_get_drvdata(pdev);
unsigned long clk_rate = aemif->clk_rate;
struct aemif_cs_data *data;
- int ret;
u32 cs;
u32 val;
@@ -275,68 +300,34 @@ static int of_aemif_parse_abus_config(struct platform_device *pdev,
aemif_get_hw_params(pdev, aemif->num_cs++);
/* override the values from device node */
- if (!of_property_read_u32(np, "ti,cs-min-turnaround-ns", &val)) {
- ret = aemif_calc_rate(pdev, val, clk_rate, TA_MAX);
- if (ret < 0)
- return ret;
-
- data->timings.ta = ret;
- }
+ if (!of_property_read_u32(np, "ti,cs-min-turnaround-ns", &val))
+ data->timings.ta = aemif_calc_rate(pdev, val, clk_rate);
- if (!of_property_read_u32(np, "ti,cs-read-hold-ns", &val)) {
- ret = aemif_calc_rate(pdev, val, clk_rate, RHOLD_MAX);
- if (ret < 0)
- return ret;
-
- data->timings.rhold = ret;
- }
+ if (!of_property_read_u32(np, "ti,cs-read-hold-ns", &val))
+ data->timings.rhold = aemif_calc_rate(pdev, val, clk_rate);
- if (!of_property_read_u32(np, "ti,cs-read-strobe-ns", &val)) {
- ret = aemif_calc_rate(pdev, val, clk_rate, RSTROBE_MAX);
- if (ret < 0)
- return ret;
+ if (!of_property_read_u32(np, "ti,cs-read-strobe-ns", &val))
+ data->timings.rstrobe = aemif_calc_rate(pdev, val, clk_rate);
- data->timings.rstrobe = ret;
- }
+ if (!of_property_read_u32(np, "ti,cs-read-setup-ns", &val))
+ data->timings.rsetup = aemif_calc_rate(pdev, val, clk_rate);
- if (!of_property_read_u32(np, "ti,cs-read-setup-ns", &val)) {
- ret = aemif_calc_rate(pdev, val, clk_rate, RSETUP_MAX);
- if (ret < 0)
- return ret;
+ if (!of_property_read_u32(np, "ti,cs-write-hold-ns", &val))
+ data->timings.whold = aemif_calc_rate(pdev, val, clk_rate);
- data->timings.rsetup = ret;
- }
+ if (!of_property_read_u32(np, "ti,cs-write-strobe-ns", &val))
+ data->timings.wstrobe = aemif_calc_rate(pdev, val, clk_rate);
- if (!of_property_read_u32(np, "ti,cs-write-hold-ns", &val)) {
- ret = aemif_calc_rate(pdev, val, clk_rate, WHOLD_MAX);
- if (ret < 0)
- return ret;
-
- data->timings.whold = ret;
- }
-
- if (!of_property_read_u32(np, "ti,cs-write-strobe-ns", &val)) {
- ret = aemif_calc_rate(pdev, val, clk_rate, WSTROBE_MAX);
- if (ret < 0)
- return ret;
-
- data->timings.wstrobe = ret;
- }
-
- if (!of_property_read_u32(np, "ti,cs-write-setup-ns", &val)) {
- ret = aemif_calc_rate(pdev, val, clk_rate, WSETUP_MAX);
- if (ret < 0)
- return ret;
-
- data->timings.wsetup = ret;
- }
+ if (!of_property_read_u32(np, "ti,cs-write-setup-ns", &val))
+ data->timings.wsetup = aemif_calc_rate(pdev, val, clk_rate);
if (!of_property_read_u32(np, "ti,cs-bus-width", &val))
if (val == 16)
data->asize = 1;
data->enable_ew = of_property_read_bool(np, "ti,cs-extended-wait-mode");
data->enable_ss = of_property_read_bool(np, "ti,cs-select-strobe-mode");
- return 0;
+
+ return aemif_check_cs_timings(&data->timings);
}
static const struct of_device_id aemif_of_match[] = {
--
2.47.0On 15/11/2024 14:26, Bastien Curutchet wrote:
> aemif_calc_rate() check the validity of a new computed timing against a
> 'max' value given as input. This isn't convenient if we want to check
> the CS timing configuration somewhere else in the code.
>
> Wrap the verification of all the chip select's timing configuration into a
> single function to ease its exportation in upcoming patches.
> Remove the 'max' input from aemif_calc_rate() as it's no longer used.
>
> Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
> ---
> drivers/memory/ti-aemif.c | 107 +++++++++++++++++---------------------
> 1 file changed, 49 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
> index aec6d6464efa..5c1c6f95185f 100644
> --- a/drivers/memory/ti-aemif.c
> +++ b/drivers/memory/ti-aemif.c
> @@ -132,18 +132,48 @@ struct aemif_device {
> struct aemif_cs_data cs_data[NUM_CS];
> };
>
> +/**
> + * aemif_check_cs_timings - Check the validity of a CS timing configuration.
> + * @timings: timings configuration
> + *
> + * @return: 0 if the timing configuration is valid, negative error number otherwise.
> + */
> +static int aemif_check_cs_timings(struct aemif_cs_timings *timings)
> +{
> + if (timings->ta > TA_MAX)
> + return -EINVAL;
> +
> + if (timings->rhold > RHOLD_MAX)
> + return -EINVAL;
> +
> + if (timings->rstrobe > RSTROBE_MAX)
> + return -EINVAL;
> +
> + if (timings->rsetup > RSETUP_MAX)
> + return -EINVAL;
> +
> + if (timings->whold > WHOLD_MAX)
> + return -EINVAL;
> +
> + if (timings->wstrobe > WSTROBE_MAX)
> + return -EINVAL;
> +
> + if (timings->wsetup > WSETUP_MAX)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> /**
> * aemif_calc_rate - calculate timing data.
> * @pdev: platform device to calculate for
> * @wanted: The cycle time needed in nanoseconds.
> * @clk: The input clock rate in kHz.
> - * @max: The maximum divider value that can be programmed.
> *
> * On success, returns the calculated timing value minus 1 for easy
> * programming into AEMIF timing registers, else negative errno.
> */
> -static int aemif_calc_rate(struct platform_device *pdev, int wanted,
> - unsigned long clk, int max)
> +static int aemif_calc_rate(struct platform_device *pdev, int wanted, unsigned long clk)
> {
> int result;
>
> @@ -156,10 +186,6 @@ static int aemif_calc_rate(struct platform_device *pdev, int wanted,
> if (result < 0)
> result = 0;
>
> - /* ... But configuring tighter timings is not an option. */
> - else if (result > max)
> - result = -EINVAL;
> -
> return result;
> }
>
> @@ -249,7 +275,6 @@ static int of_aemif_parse_abus_config(struct platform_device *pdev,
> struct aemif_device *aemif = platform_get_drvdata(pdev);
> unsigned long clk_rate = aemif->clk_rate;
> struct aemif_cs_data *data;
> - int ret;
> u32 cs;
> u32 val;
>
> @@ -275,68 +300,34 @@ static int of_aemif_parse_abus_config(struct platform_device *pdev,
> aemif_get_hw_params(pdev, aemif->num_cs++);
>
> /* override the values from device node */
> - if (!of_property_read_u32(np, "ti,cs-min-turnaround-ns", &val)) {
> - ret = aemif_calc_rate(pdev, val, clk_rate, TA_MAX);
> - if (ret < 0)
> - return ret;
> -
> - data->timings.ta = ret;
> - }
> + if (!of_property_read_u32(np, "ti,cs-min-turnaround-ns", &val))
> + data->timings.ta = aemif_calc_rate(pdev, val, clk_rate);
>
You just changed these lines in patch #1. Basically this is partial
revert of #1.
Best regards,
Krzysztof
Hi Krzysztof,
On 11/21/24 11:28 AM, Krzysztof Kozlowski wrote:
> On 15/11/2024 14:26, Bastien Curutchet wrote:
>> aemif_calc_rate() check the validity of a new computed timing against a
>> 'max' value given as input. This isn't convenient if we want to check
>> the CS timing configuration somewhere else in the code.
>>
>> Wrap the verification of all the chip select's timing configuration into a
>> single function to ease its exportation in upcoming patches.
>> Remove the 'max' input from aemif_calc_rate() as it's no longer used.
>>
>> Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
>> ---
>> drivers/memory/ti-aemif.c | 107 +++++++++++++++++---------------------
>> 1 file changed, 49 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
>> index aec6d6464efa..5c1c6f95185f 100644
>> --- a/drivers/memory/ti-aemif.c
>> +++ b/drivers/memory/ti-aemif.c
>> @@ -132,18 +132,48 @@ struct aemif_device {
>> struct aemif_cs_data cs_data[NUM_CS];
>> };
>>
>> +/**
>> + * aemif_check_cs_timings - Check the validity of a CS timing configuration.
>> + * @timings: timings configuration
>> + *
>> + * @return: 0 if the timing configuration is valid, negative error number otherwise.
>> + */
>> +static int aemif_check_cs_timings(struct aemif_cs_timings *timings)
>> +{
>> + if (timings->ta > TA_MAX)
>> + return -EINVAL;
>> +
>> + if (timings->rhold > RHOLD_MAX)
>> + return -EINVAL;
>> +
>> + if (timings->rstrobe > RSTROBE_MAX)
>> + return -EINVAL;
>> +
>> + if (timings->rsetup > RSETUP_MAX)
>> + return -EINVAL;
>> +
>> + if (timings->whold > WHOLD_MAX)
>> + return -EINVAL;
>> +
>> + if (timings->wstrobe > WSTROBE_MAX)
>> + return -EINVAL;
>> +
>> + if (timings->wsetup > WSETUP_MAX)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * aemif_calc_rate - calculate timing data.
>> * @pdev: platform device to calculate for
>> * @wanted: The cycle time needed in nanoseconds.
>> * @clk: The input clock rate in kHz.
>> - * @max: The maximum divider value that can be programmed.
>> *
>> * On success, returns the calculated timing value minus 1 for easy
>> * programming into AEMIF timing registers, else negative errno.
>> */
>> -static int aemif_calc_rate(struct platform_device *pdev, int wanted,
>> - unsigned long clk, int max)
>> +static int aemif_calc_rate(struct platform_device *pdev, int wanted, unsigned long clk)
>> {
>> int result;
>>
>> @@ -156,10 +186,6 @@ static int aemif_calc_rate(struct platform_device *pdev, int wanted,
>> if (result < 0)
>> result = 0;
>>
>> - /* ... But configuring tighter timings is not an option. */
>> - else if (result > max)
>> - result = -EINVAL;
>> -
>> return result;
>> }
>>
>> @@ -249,7 +275,6 @@ static int of_aemif_parse_abus_config(struct platform_device *pdev,
>> struct aemif_device *aemif = platform_get_drvdata(pdev);
>> unsigned long clk_rate = aemif->clk_rate;
>> struct aemif_cs_data *data;
>> - int ret;
>> u32 cs;
>> u32 val;
>>
>> @@ -275,68 +300,34 @@ static int of_aemif_parse_abus_config(struct platform_device *pdev,
>> aemif_get_hw_params(pdev, aemif->num_cs++);
>>
>> /* override the values from device node */
>> - if (!of_property_read_u32(np, "ti,cs-min-turnaround-ns", &val)) {
>> - ret = aemif_calc_rate(pdev, val, clk_rate, TA_MAX);
>> - if (ret < 0)
>> - return ret;
>> -
>> - data->timings.ta = ret;
>> - }
>> + if (!of_property_read_u32(np, "ti,cs-min-turnaround-ns", &val))
>> + data->timings.ta = aemif_calc_rate(pdev, val, clk_rate);
>>
>
> You just changed these lines in patch #1. Basically this is partial
> revert of #1.
>
IMHO this isn't a partial revert of patch #1. Patch #1 moved the call of
aemif_calc_rate() from aemif_config_abus() to here. Then, this patch
removes the check of the aemif_calc_rate() as it no longer returns
negative values. Maybe I should change the aemif_calc_rate() return type
to u32 by the way.
Best regards,
Bastien
On 15/11/2024 at 14:26:25 +01, Bastien Curutchet <bastien.curutchet@bootlin.com> wrote:
> aemif_calc_rate() check the validity of a new computed timing against a
checks
> 'max' value given as input. This isn't convenient if we want to check
> the CS timing configuration somewhere else in the code.
>
> Wrap the verification of all the chip select's timing configuration into a
> single function to ease its exportation in upcoming patches.
> Remove the 'max' input from aemif_calc_rate() as it's no longer used.
>
> Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
© 2016 - 2024 Red Hat, Inc.