mbox series

[v3,0/4] spi support for Exynos Auto v9 SoC

Message ID 20220629102304.65712-1-chanho61.park@samsung.com
Headers show
Series spi support for Exynos Auto v9 SoC | expand

Message

Chanho Park June 29, 2022, 10:23 a.m. UTC 
Add to support Exynos Auto v9 SoC's spi. By supporting USI(Universal
Serial Interface) mode, the SoC can support up to 12 spi ports. Thus, we
need to increase MAX_SPI_PORTS from 6 to 12. The spi of the SoC can
support loopback mode unlike previous exynos SoCs. To separate the
feature, we need to add .has_loopback to the s3c64xx_spi_port_config.
Furthermore, it uses 4 as the default internal clock divider. We also
need to clk_div field of the structure and assign "2" as the default
value to the existing SoC's port config.
Device tree definitions of exynosautov9-spi will be added in separated
patchset to include usi(i2c/uart/spi) nodes all together.

Changes from v2:
- Rebase the patches on top of the latest next/master (next-20220629)
- Add Andy's R-B tags for #1, #3 and #4 patches
- Add Krzysztof's R-B tag for #4 patch
- Drop div local variable assignment as suggested by Krzysztof
- Change the data type of 'div' local variables to be consistent with
  clk_div (Pointed by Andy)

Changes from v1:
- Patch #1 "increase MAX_SPI_PORTS to 12" has been squashed to the patch #4
- Add Krzysztof's RB tags for #1 and #3 patches
- Assign clk_div value to 2 for existing SoC's port configs
- Make const of exynosautov9_spi_port_config

Chanho Park (4):
  spi: s3c64xx: support loopback mode
  spi: s3c64xx: support custom value of internal clock divider
  dt-bindings: samsung,spi: define exynosautov9 compatible
  spi: s3c64xx: add spi port configuration for Exynos Auto v9 SoC

 .../devicetree/bindings/spi/samsung,spi.yaml  |  5 +-
 drivers/spi/spi-s3c64xx.c                     | 54 +++++++++++++++----
 2 files changed, 49 insertions(+), 10 deletions(-)

Comments

Krzysztof Kozlowski June 29, 2022, 11:13 a.m. UTC | #1 
On 29/06/2022 12:23, Chanho Park wrote:
> Modern exynos SoCs such as Exynos Auto v9 have different internal clock
> divider, for example "4". To support this internal value, this adds
> clk_div of the s3c64xx_spi_port_config and assign "2" as the default
> value to existing s3c64xx_spi_port_config.
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof
Mark Brown June 29, 2022, 11:36 a.m. UTC | #2 
On Wed, Jun 29, 2022 at 07:23:02PM +0900, Chanho Park wrote:
> Modern exynos SoCs such as Exynos Auto v9 have different internal clock
> divider, for example "4". To support this internal value, this adds
> clk_div of the s3c64xx_spi_port_config and assign "2" as the default
> value to existing s3c64xx_spi_port_config.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.
Linus Walleij June 30, 2022, 9:07 a.m. UTC | #3 
On Wed, Jun 29, 2022 at 12:27 PM Chanho Park <chanho61.park@samsung.com> wrote:

> Modern exynos SoCs such as Exynos Auto v9 have different internal clock
> divider, for example "4". To support this internal value, this adds
> clk_div of the s3c64xx_spi_port_config and assign "2" as the default
> value to existing s3c64xx_spi_port_config.
>
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>

I don't really see why this divider value should be hard-coded like this.

I guess it is some default value, that's OK I guess, then call it:

> + * @clk_div: Internal clock divider
> +       int     clk_div;

clk_div_default

And the documentation should say "clock divider to be used
by default unless a specific clock frequency is configured"

Yours,
Linus Walleij