2022-08-25 15:04:28

by Shreeya Patel

[permalink] [raw]
Subject: [PATCH v3] spi: amd: Configure device speed

From: Lucas Tanure <[email protected]>

Number of clock frequencies are supported by AMD controller
which are mentioned in the amd_spi_freq structure table.

Create mechanism to configure device clock frequency such
that it is strictly less than the requested frequency.

Give priority to the device transfer speed and in case
it is not set then use the max clock speed supported
by the device.

Signed-off-by: Lucas Tanure <[email protected]>
Co-developed-by: Shreeya Patel <[email protected]>
Signed-off-by: Shreeya Patel <[email protected]>
---

Changes in v3
- Make changes as per the current code and resend.

Changes in v2
- Improve the commit message.

drivers/spi/spi-amd.c | 97 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index b3b3f5167c4c..264633c5ce0b 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -36,6 +36,18 @@
#define AMD_SPI_FIFO_SIZE 70
#define AMD_SPI_MEM_SIZE 200

+#define AMD_SPI_ENA_REG 0x20
+#define AMD_SPI_ALT_SPD_SHIFT 20
+#define AMD_SPI_ALT_SPD_MASK GENMASK(23, AMD_SPI_ALT_SPD_SHIFT)
+#define AMD_SPI_SPI100_SHIFT 0
+#define AMD_SPI_SPI100_MASK GENMASK(AMD_SPI_SPI100_SHIFT, AMD_SPI_SPI100_SHIFT)
+#define AMD_SPI_SPEED_REG 0x6C
+#define AMD_SPI_SPD7_SHIFT 8
+#define AMD_SPI_SPD7_MASK GENMASK(13, AMD_SPI_SPD7_SHIFT)
+
+#define AMD_SPI_MAX_HZ 100000000
+#define AMD_SPI_MIN_HZ 800000
+
/**
* enum amd_spi_versions - SPI controller versions
* @AMD_SPI_V1: AMDI0061 hardware version
@@ -46,14 +58,41 @@ enum amd_spi_versions {
AMD_SPI_V2,
};

+enum amd_spi_speed {
+ F_66_66MHz,
+ F_33_33MHz,
+ F_22_22MHz,
+ F_16_66MHz,
+ F_100MHz,
+ F_800KHz,
+ SPI_SPD7,
+ F_50MHz = 0x4,
+ F_4MHz = 0x32,
+ F_3_17MHz = 0x3F
+};
+
+/**
+ * struct amd_spi_freq - Matches device speed with values to write in regs
+ * @speed_hz: Device frequency
+ * @enable_val: Value to be written to "enable register"
+ * @spd7_val: Some frequencies requires to have a value written at SPISPEED register
+ */
+struct amd_spi_freq {
+ u32 speed_hz;
+ u32 enable_val;
+ u32 spd7_val;
+};
+
/**
* struct amd_spi - SPI driver instance
* @io_remap_addr: Start address of the SPI controller registers
* @version: SPI controller hardware version
+ * @speed_hz: Device frequency
*/
struct amd_spi {
void __iomem *io_remap_addr;
enum amd_spi_versions version;
+ unsigned int speed_hz;
};

static inline u8 amd_spi_readreg8(struct amd_spi *amd_spi, int idx)
@@ -185,11 +224,62 @@ static int amd_spi_master_setup(struct spi_device *spi)
return 0;
}

+static const struct amd_spi_freq amd_spi_freq[] = {
+ { AMD_SPI_MAX_HZ, F_100MHz, 0},
+ { 66660000, F_66_66MHz, 0},
+ { 50000000, SPI_SPD7, F_50MHz},
+ { 33330000, F_33_33MHz, 0},
+ { 22220000, F_22_22MHz, 0},
+ { 16660000, F_16_66MHz, 0},
+ { 4000000, SPI_SPD7, F_4MHz},
+ { 3170000, SPI_SPD7, F_3_17MHz},
+ { AMD_SPI_MIN_HZ, F_800KHz, 0},
+};
+
+static int amd_set_spi_freq(struct amd_spi *amd_spi, u32 speed_hz)
+{
+ unsigned int i, spd7_val, alt_spd;
+
+ if (speed_hz == amd_spi->speed_hz)
+ return 0;
+
+ if (speed_hz < AMD_SPI_MIN_HZ)
+ return -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(amd_spi_freq); i++)
+ if (speed_hz >= amd_spi_freq[i].speed_hz)
+ break;
+
+ if (speed_hz == amd_spi_freq[i].speed_hz)
+ return 0;
+
+ amd_spi->speed_hz = amd_spi_freq[i].speed_hz;
+
+ alt_spd = (amd_spi_freq[i].enable_val << AMD_SPI_ALT_SPD_SHIFT)
+ & AMD_SPI_ALT_SPD_MASK;
+ amd_spi_setclear_reg32(amd_spi, AMD_SPI_ENA_REG, alt_spd,
+ AMD_SPI_ALT_SPD_MASK);
+
+ if (amd_spi->speed_hz == AMD_SPI_MAX_HZ)
+ amd_spi_setclear_reg32(amd_spi, AMD_SPI_ENA_REG, 1,
+ AMD_SPI_SPI100_MASK);
+
+ if (amd_spi_freq[i].spd7_val) {
+ spd7_val = (amd_spi_freq[i].spd7_val << AMD_SPI_SPD7_SHIFT)
+ & AMD_SPI_SPD7_MASK;
+ amd_spi_setclear_reg32(amd_spi, AMD_SPI_SPEED_REG, spd7_val,
+ AMD_SPI_SPD7_MASK);
+ }
+
+ return 0;
+}
+
static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
struct spi_master *master,
struct spi_message *message)
{
struct spi_transfer *xfer = NULL;
+ struct spi_device *spi = message->spi;
u8 cmd_opcode = 0, fifo_pos = AMD_SPI_FIFO_BASE;
u8 *buf = NULL;
u32 i = 0;
@@ -197,6 +287,11 @@ static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,

list_for_each_entry(xfer, &message->transfers,
transfer_list) {
+ if (xfer->speed_hz)
+ amd_set_spi_freq(amd_spi, xfer->speed_hz);
+ else
+ amd_set_spi_freq(amd_spi, spi->max_speed_hz);
+
if (xfer->tx_buf) {
buf = (u8 *)xfer->tx_buf;
if (!tx_len) {
@@ -313,6 +408,8 @@ static int amd_spi_probe(struct platform_device *pdev)
master->num_chipselect = 4;
master->mode_bits = 0;
master->flags = SPI_MASTER_HALF_DUPLEX;
+ master->max_speed_hz = AMD_SPI_MAX_HZ;
+ master->min_speed_hz = AMD_SPI_MIN_HZ;
master->setup = amd_spi_master_setup;
master->transfer_one_message = amd_spi_master_transfer;
master->max_transfer_size = amd_spi_max_transfer_size;
--
2.30.2


2022-08-25 17:03:59

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v3] spi: amd: Configure device speed

On Thu, 25 Aug 2022 20:01:32 +0530, Shreeya Patel wrote:
> From: Lucas Tanure <[email protected]>
>
> Number of clock frequencies are supported by AMD controller
> which are mentioned in the amd_spi_freq structure table.
>
> Create mechanism to configure device clock frequency such
> that it is strictly less than the requested frequency.
>
> [...]

Applied to

broonie/spi.git for-next

Thanks!

[1/1] spi: amd: Configure device speed
commit: 3fe26121dc3a9bf64e18fe0075cd9a92c9cd1b1a

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

2022-08-26 06:58:09

by Christophe JAILLET

[permalink] [raw]
Subject: Re: [PATCH v3] spi: amd: Configure device speed

Le 25/08/2022 à 16:31, Shreeya Patel a écrit :
> From: Lucas Tanure <tanureal-yzvPICuk2AA4QjBA90+/[email protected]>
>
> Number of clock frequencies are supported by AMD controller
> which are mentioned in the amd_spi_freq structure table.
>
> Create mechanism to configure device clock frequency such
> that it is strictly less than the requested frequency.
>
> Give priority to the device transfer speed and in case
> it is not set then use the max clock speed supported
> by the device.
>
> Signed-off-by: Lucas Tanure <tanureal-yzvPICuk2AA4QjBA90+/[email protected]>
> Co-developed-by: Shreeya Patel <shreeya.patel-ZGY8ohtN/[email protected]>
> Signed-off-by: Shreeya Patel <shreeya.patel-ZGY8ohtN/[email protected]>
> ---
>
> Changes in v3
> - Make changes as per the current code and resend.
>
> Changes in v2
> - Improve the commit message.
>
> drivers/spi/spi-amd.c | 97 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 97 insertions(+)
>
> diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
> index b3b3f5167c4c..264633c5ce0b 100644
> --- a/drivers/spi/spi-amd.c
> +++ b/drivers/spi/spi-amd.c
> @@ -36,6 +36,18 @@
> #define AMD_SPI_FIFO_SIZE 70
> #define AMD_SPI_MEM_SIZE 200
>
> +#define AMD_SPI_ENA_REG 0x20
> +#define AMD_SPI_ALT_SPD_SHIFT 20
> +#define AMD_SPI_ALT_SPD_MASK GENMASK(23, AMD_SPI_ALT_SPD_SHIFT)
> +#define AMD_SPI_SPI100_SHIFT 0
> +#define AMD_SPI_SPI100_MASK GENMASK(AMD_SPI_SPI100_SHIFT, AMD_SPI_SPI100_SHIFT)
> +#define AMD_SPI_SPEED_REG 0x6C
> +#define AMD_SPI_SPD7_SHIFT 8
> +#define AMD_SPI_SPD7_MASK GENMASK(13, AMD_SPI_SPD7_SHIFT)
> +
> +#define AMD_SPI_MAX_HZ 100000000
> +#define AMD_SPI_MIN_HZ 800000
> +
> /**
> * enum amd_spi_versions - SPI controller versions
> * @AMD_SPI_V1: AMDI0061 hardware version
> @@ -46,14 +58,41 @@ enum amd_spi_versions {
> AMD_SPI_V2,
> };
>
> +enum amd_spi_speed {
> + F_66_66MHz,
> + F_33_33MHz,
> + F_22_22MHz,
> + F_16_66MHz,
> + F_100MHz,
> + F_800KHz,
> + SPI_SPD7,
> + F_50MHz = 0x4,
> + F_4MHz = 0x32,
> + F_3_17MHz = 0x3F
> +};
> +
> +/**
> + * struct amd_spi_freq - Matches device speed with values to write in regs
> + * @speed_hz: Device frequency
> + * @enable_val: Value to be written to "enable register"
> + * @spd7_val: Some frequencies requires to have a value written at SPISPEED register
> + */
> +struct amd_spi_freq {
> + u32 speed_hz;
> + u32 enable_val;
> + u32 spd7_val;
> +};
> +
> /**
> * struct amd_spi - SPI driver instance
> * @io_remap_addr: Start address of the SPI controller registers
> * @version: SPI controller hardware version
> + * @speed_hz: Device frequency
> */
> struct amd_spi {
> void __iomem *io_remap_addr;
> enum amd_spi_versions version;
> + unsigned int speed_hz;
> };
>
> static inline u8 amd_spi_readreg8(struct amd_spi *amd_spi, int idx)
> @@ -185,11 +224,62 @@ static int amd_spi_master_setup(struct spi_device *spi)
> return 0;
> }
>
> +static const struct amd_spi_freq amd_spi_freq[] = {
> + { AMD_SPI_MAX_HZ, F_100MHz, 0},
> + { 66660000, F_66_66MHz, 0},
> + { 50000000, SPI_SPD7, F_50MHz},
> + { 33330000, F_33_33MHz, 0},
> + { 22220000, F_22_22MHz, 0},
> + { 16660000, F_16_66MHz, 0},
> + { 4000000, SPI_SPD7, F_4MHz},
> + { 3170000, SPI_SPD7, F_3_17MHz},
> + { AMD_SPI_MIN_HZ, F_800KHz, 0},
> +};
> +

Hi,
I'm not sure to understand the logic here:

> +static int amd_set_spi_freq(struct amd_spi *amd_spi, u32 speed_hz)
> +{
> + unsigned int i, spd7_val, alt_spd;
> +
> + if (speed_hz == amd_spi->speed_hz)
> + return 0;

If we set a speed that is already the one in use --> return success

> +
> + if (speed_hz < AMD_SPI_MIN_HZ)
> + return -EINVAL;

If we request a speed below the lower possible --> return error.

> +
> + for (i = 0; i < ARRAY_SIZE(amd_spi_freq); i++)
> + if (speed_hz >= amd_spi_freq[i].speed_hz)
> + break;

Search the array for the speed that is just below or equal to the
requested one.

> +
> + if (speed_hz == amd_spi_freq[i].speed_hz)
> + return 0;

If it is an exact match --> return success
???

So, if the requested value is one of the table, nothing is done and it
can't be selected.

Is it what is expected?
Without looking at the rest of the code or at the driver, I don't see
the point of this test.


Should this be:
if (amd_spi->speed_hz == amd_spi_freq[i].speed_hz)
^^^^^^^^^
?


If so, the first 'if (speed_hz == amd_spi->speed_hz)' looks redundant.
(I guess it is a slow path, so should it be an optimization, it
should'nt really matter)

CJ

> +
> + amd_spi->speed_hz = amd_spi_freq[i].speed_hz;
> +
> + alt_spd = (amd_spi_freq[i].enable_val << AMD_SPI_ALT_SPD_SHIFT)
> + & AMD_SPI_ALT_SPD_MASK;
> + amd_spi_setclear_reg32(amd_spi, AMD_SPI_ENA_REG, alt_spd,
> + AMD_SPI_ALT_SPD_MASK);
> +
> + if (amd_spi->speed_hz == AMD_SPI_MAX_HZ)
> + amd_spi_setclear_reg32(amd_spi, AMD_SPI_ENA_REG, 1,
> + AMD_SPI_SPI100_MASK);
> +
> + if (amd_spi_freq[i].spd7_val) {
> + spd7_val = (amd_spi_freq[i].spd7_val << AMD_SPI_SPD7_SHIFT)
> + & AMD_SPI_SPD7_MASK;
> + amd_spi_setclear_reg32(amd_spi, AMD_SPI_SPEED_REG, spd7_val,
> + AMD_SPI_SPD7_MASK);
> + }
> +
> + return 0;
> +}
> +
> static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
> struct spi_master *master,
> struct spi_message *message)
> {
> struct spi_transfer *xfer = NULL;
> + struct spi_device *spi = message->spi;
> u8 cmd_opcode = 0, fifo_pos = AMD_SPI_FIFO_BASE;
> u8 *buf = NULL;
> u32 i = 0;
> @@ -197,6 +287,11 @@ static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
>
> list_for_each_entry(xfer, &message->transfers,
> transfer_list) {
> + if (xfer->speed_hz)
> + amd_set_spi_freq(amd_spi, xfer->speed_hz);
> + else
> + amd_set_spi_freq(amd_spi, spi->max_speed_hz);
> +
> if (xfer->tx_buf) {
> buf = (u8 *)xfer->tx_buf;
> if (!tx_len) {
> @@ -313,6 +408,8 @@ static int amd_spi_probe(struct platform_device *pdev)
> master->num_chipselect = 4;
> master->mode_bits = 0;
> master->flags = SPI_MASTER_HALF_DUPLEX;
> + master->max_speed_hz = AMD_SPI_MAX_HZ;
> + master->min_speed_hz = AMD_SPI_MIN_HZ;
> master->setup = amd_spi_master_setup;
> master->transfer_one_message = amd_spi_master_transfer;
> master->max_transfer_size = amd_spi_max_transfer_size;

2022-08-26 11:37:49

by Shreeya Patel

[permalink] [raw]
Subject: Re: [PATCH v3] spi: amd: Configure device speed


On 26/08/22 12:14, Christophe JAILLET wrote:
> Le 25/08/2022 à 16:31, Shreeya Patel a écrit :
>> From: Lucas Tanure
>> <tanureal-yzvPICuk2AA4QjBA90+/[email protected]>
>>
>> Number of clock frequencies are supported by AMD controller
>> which are mentioned in the amd_spi_freq structure table.
>>
>> Create mechanism to configure device clock frequency such
>> that it is strictly less than the requested frequency.
>>
>> Give priority to the device transfer speed and in case
>> it is not set then use the max clock speed supported
>> by the device.
>>
>> Signed-off-by: Lucas Tanure
>> <tanureal-yzvPICuk2AA4QjBA90+/[email protected]>
>> Co-developed-by: Shreeya Patel
>> <shreeya.patel-ZGY8ohtN/[email protected]>
>> Signed-off-by: Shreeya Patel
>> <shreeya.patel-ZGY8ohtN/[email protected]>
>> ---
>>
>> Changes in v3
>>    - Make changes as per the current code and resend.
>>
>> Changes in v2
>>    - Improve the commit message.
>>
>>   drivers/spi/spi-amd.c | 97 +++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 97 insertions(+)
>>
>> diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
>> index b3b3f5167c4c..264633c5ce0b 100644
>> --- a/drivers/spi/spi-amd.c
>> +++ b/drivers/spi/spi-amd.c
>> @@ -36,6 +36,18 @@
>>   #define AMD_SPI_FIFO_SIZE    70
>>   #define AMD_SPI_MEM_SIZE    200
>>   +#define AMD_SPI_ENA_REG        0x20
>> +#define AMD_SPI_ALT_SPD_SHIFT    20
>> +#define AMD_SPI_ALT_SPD_MASK    GENMASK(23, AMD_SPI_ALT_SPD_SHIFT)
>> +#define AMD_SPI_SPI100_SHIFT    0
>> +#define AMD_SPI_SPI100_MASK    GENMASK(AMD_SPI_SPI100_SHIFT,
>> AMD_SPI_SPI100_SHIFT)
>> +#define AMD_SPI_SPEED_REG    0x6C
>> +#define AMD_SPI_SPD7_SHIFT    8
>> +#define AMD_SPI_SPD7_MASK    GENMASK(13, AMD_SPI_SPD7_SHIFT)
>> +
>> +#define AMD_SPI_MAX_HZ        100000000
>> +#define AMD_SPI_MIN_HZ        800000
>> +
>>   /**
>>    * enum amd_spi_versions - SPI controller versions
>>    * @AMD_SPI_V1:        AMDI0061 hardware version
>> @@ -46,14 +58,41 @@ enum amd_spi_versions {
>>       AMD_SPI_V2,
>>   };
>>   +enum amd_spi_speed {
>> +    F_66_66MHz,
>> +    F_33_33MHz,
>> +    F_22_22MHz,
>> +    F_16_66MHz,
>> +    F_100MHz,
>> +    F_800KHz,
>> +    SPI_SPD7,
>> +    F_50MHz = 0x4,
>> +    F_4MHz = 0x32,
>> +    F_3_17MHz = 0x3F
>> +};
>> +
>> +/**
>> + * struct amd_spi_freq - Matches device speed with values to write
>> in regs
>> + * @speed_hz: Device frequency
>> + * @enable_val: Value to be written to "enable register"
>> + * @spd7_val: Some frequencies requires to have a value written at
>> SPISPEED register
>> + */
>> +struct amd_spi_freq {
>> +    u32 speed_hz;
>> +    u32 enable_val;
>> +    u32 spd7_val;
>> +};
>> +
>>   /**
>>    * struct amd_spi - SPI driver instance
>>    * @io_remap_addr:    Start address of the SPI controller registers
>>    * @version:        SPI controller hardware version
>> + * @speed_hz:        Device frequency
>>    */
>>   struct amd_spi {
>>       void __iomem *io_remap_addr;
>>       enum amd_spi_versions version;
>> +    unsigned int speed_hz;
>>   };
>>     static inline u8 amd_spi_readreg8(struct amd_spi *amd_spi, int idx)
>> @@ -185,11 +224,62 @@ static int amd_spi_master_setup(struct
>> spi_device *spi)
>>       return 0;
>>   }
>>   +static const struct amd_spi_freq amd_spi_freq[] = {
>> +    { AMD_SPI_MAX_HZ,   F_100MHz,         0},
>> +    {       66660000, F_66_66MHz,         0},
>> +    {       50000000,   SPI_SPD7,   F_50MHz},
>> +    {       33330000, F_33_33MHz,         0},
>> +    {       22220000, F_22_22MHz,         0},
>> +    {       16660000, F_16_66MHz,         0},
>> +    {        4000000,   SPI_SPD7,    F_4MHz},
>> +    {        3170000,   SPI_SPD7, F_3_17MHz},
>> +    { AMD_SPI_MIN_HZ,   F_800KHz,         0},
>> +};
>> +
>
> Hi,
> I'm not sure to understand the logic here:
>
>> +static int amd_set_spi_freq(struct amd_spi *amd_spi, u32 speed_hz)
>> +{
>> +    unsigned int i, spd7_val, alt_spd;
>> +
>> +    if (speed_hz == amd_spi->speed_hz)
>> +        return 0;
>
> If we set a speed that is already the one in use --> return success
>
>> +
>> +    if (speed_hz < AMD_SPI_MIN_HZ)
>> +        return -EINVAL;
>
> If we request a speed below the lower possible --> return error.
>
>> +
>> +    for (i = 0; i < ARRAY_SIZE(amd_spi_freq); i++)
>> +        if (speed_hz >= amd_spi_freq[i].speed_hz)
>> +            break;
>
> Search the array for the speed that is just below or equal to the
> requested one.
>
>> +
>> +    if (speed_hz == amd_spi_freq[i].speed_hz)
>> +        return 0;
>
> If it is an exact match --> return success
> ???
>
> So, if the requested value is one of the table, nothing is done and it
> can't be selected.
>
> Is it what is expected?
> Without looking at the rest of the code or at the driver, I don't see
> the point of this test.
>
>
> Should this be:
>     if (amd_spi->speed_hz == amd_spi_freq[i].speed_hz)
>         ^^^^^^^^^
> ?
>

Yes you are right, it should be amd_spi->speed_hz instead of speed_hz.
Thanks for pointing it out.
I'll send a fix for it.


Thanks,
Shreeya Patel

>
> If so, the first 'if (speed_hz == amd_spi->speed_hz)' looks redundant.
> (I guess it is a slow path, so should it be an optimization, it
> should'nt really matter)
>
> CJ
>
>> +
>> +    amd_spi->speed_hz = amd_spi_freq[i].speed_hz;
>> +
>> +    alt_spd = (amd_spi_freq[i].enable_val << AMD_SPI_ALT_SPD_SHIFT)
>> +           & AMD_SPI_ALT_SPD_MASK;
>> +    amd_spi_setclear_reg32(amd_spi, AMD_SPI_ENA_REG, alt_spd,
>> +                   AMD_SPI_ALT_SPD_MASK);
>> +
>> +    if (amd_spi->speed_hz == AMD_SPI_MAX_HZ)
>> +        amd_spi_setclear_reg32(amd_spi, AMD_SPI_ENA_REG, 1,
>> +                       AMD_SPI_SPI100_MASK);
>> +
>> +    if (amd_spi_freq[i].spd7_val) {
>> +        spd7_val = (amd_spi_freq[i].spd7_val << AMD_SPI_SPD7_SHIFT)
>> +                & AMD_SPI_SPD7_MASK;
>> +        amd_spi_setclear_reg32(amd_spi, AMD_SPI_SPEED_REG, spd7_val,
>> +                       AMD_SPI_SPD7_MASK);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
>>                       struct spi_master *master,
>>                       struct spi_message *message)
>>   {
>>       struct spi_transfer *xfer = NULL;
>> +    struct spi_device *spi = message->spi;
>>       u8 cmd_opcode = 0, fifo_pos = AMD_SPI_FIFO_BASE;
>>       u8 *buf = NULL;
>>       u32 i = 0;
>> @@ -197,6 +287,11 @@ static inline int amd_spi_fifo_xfer(struct
>> amd_spi *amd_spi,
>>         list_for_each_entry(xfer, &message->transfers,
>>                   transfer_list) {
>> +        if (xfer->speed_hz)
>> +            amd_set_spi_freq(amd_spi, xfer->speed_hz);
>> +        else
>> +            amd_set_spi_freq(amd_spi, spi->max_speed_hz);
>> +
>>           if (xfer->tx_buf) {
>>               buf = (u8 *)xfer->tx_buf;
>>               if (!tx_len) {
>> @@ -313,6 +408,8 @@ static int amd_spi_probe(struct platform_device
>> *pdev)
>>       master->num_chipselect = 4;
>>       master->mode_bits = 0;
>>       master->flags = SPI_MASTER_HALF_DUPLEX;
>> +    master->max_speed_hz = AMD_SPI_MAX_HZ;
>> +    master->min_speed_hz = AMD_SPI_MIN_HZ;
>>       master->setup = amd_spi_master_setup;
>>       master->transfer_one_message = amd_spi_master_transfer;
>>       master->max_transfer_size = amd_spi_max_transfer_size;
>
>

2022-08-26 18:28:16

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v3] spi: amd: Configure device speed

On Fri, Aug 26, 2022 at 08:44:31AM +0200, Christophe JAILLET wrote:

> So, if the requested value is one of the table, nothing is done and it can't
> be selected.

> Is it what is expected?
> Without looking at the rest of the code or at the driver, I don't see the
> point of this test.

No, the speed should be selected.


Attachments:
(No filename) (333.00 B)
signature.asc (499.00 B)
Download all attachments