Re: [PATCH] ARM: dts: at91: sama7g5: fix clock id for ebi node

[Claudiu.Beznea]

On 23.02.2023 13:28, Mihai Sain wrote:
> The PMC_MCK1 clock id for the ebi node is 23.
> 
> Fixes: 4ce85577ac ("ARM: dts: at91: sama7g5/sama7g5ek: align with Linux DT")
> Signed-off-by: Mihai Sain 

Reviewed-by: Claudiu Beznea 


> ---
>  arch/arm/dts/sama7g5.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/dts/sama7g5.dtsi b/arch/arm/dts/sama7g5.dtsi
> index d38090d7dd..a802153af8 100644
> --- a/arch/arm/dts/sama7g5.dtsi
> +++ b/arch/arm/dts/sama7g5.dtsi
> @@ -150,7 +150,7 @@
> 0x1 0x0 0x4800 0x800
> 0x2 0x0 0x5000 0x800
> 0x3 0x0 0x5800 0x800>;
> - clocks = < PMC_TYPE_CORE 13>; /* PMC_MCK1 */
> + clocks = < PMC_TYPE_CORE 23>; /* PMC_MCK1 */
>   status = "disabled";
>  
>   nand_controller: nand-controller {

Re: [PATCH 70/88] sysreset: at91: Correct Makefile rule for SYSRESET_AT91

[Claudiu.Beznea]

On 24.01.2023 00:00, Simon Glass wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> The SPL_TPL part is in the wrong place. Fix it.
> 
> Signed-off-by: Simon Glass 
> Fixes: 71d4393f846 ("sysreset: Add Atmel/Microchip sysreset driver")

Reviewed-by: Claudiu Beznea 


> ---
> 
>  drivers/sysreset/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
> index 0ed3bbf356a..40c876764af 100644
> --- a/drivers/sysreset/Makefile
> +++ b/drivers/sysreset/Makefile
> @@ -20,6 +20,6 @@ obj-$(CONFIG_SYSRESET_TI_SCI) += sysreset-ti-sci.o
>  obj-$(CONFIG_SYSRESET_SYSCON) += sysreset_syscon.o
>  obj-$(CONFIG_SYSRESET_WATCHDOG) += sysreset_watchdog.o
>  obj-$(CONFIG_SYSRESET_RESETCTL) += sysreset_resetctl.o
> -obj-$(CONFIG_SYSRESET_$(SPL_TPL_)AT91) += sysreset_at91.o
> +obj-$(CONFIG_$(SPL_TPL_)SYSRESET_AT91) += sysreset_at91.o
>  obj-$(CONFIG_$(SPL_TPL_)SYSRESET_X86) += sysreset_x86.o
>  obj-$(CONFIG_TARGET_XTFPGA) += sysreset_xtfpga.o
> --
> 2.39.1.456.gfc5497dd1b-goog
> 

Re: [PATCH 5/5] configs: at91: sam9x60: Add required configs for the USB clock

[Claudiu.Beznea]

On 23.12.2022 14:33, Sergiu Moga wrote:
> Add the configs required to use the SAM9X60's USB clock.
> 
> Signed-off-by: Sergiu Moga 

Reviewed-by: Claudiu Beznea 


> ---
>  configs/sam9x60_curiosity_mmc_defconfig | 1 +
>  configs/sam9x60ek_mmc_defconfig | 1 +
>  configs/sam9x60ek_nandflash_defconfig   | 1 +
>  configs/sam9x60ek_qspiflash_defconfig   | 1 +
>  4 files changed, 4 insertions(+)
> 
> diff --git a/configs/sam9x60_curiosity_mmc_defconfig 
> b/configs/sam9x60_curiosity_mmc_defconfig
> index 732b5adf26..2bb196a5b6 100644
> --- a/configs/sam9x60_curiosity_mmc_defconfig
> +++ b/configs/sam9x60_curiosity_mmc_defconfig
> @@ -54,6 +54,7 @@ CONFIG_CLK_CCF=y
>  CONFIG_CLK_AT91=y
>  CONFIG_AT91_GENERIC_CLK=y
>  CONFIG_AT91_SAM9X60_PLL=y
> +CONFIG_AT91_SAM9X60_USB=y
>  CONFIG_CPU=y
>  CONFIG_AT91_GPIO=y
>  CONFIG_DM_I2C=y
> diff --git a/configs/sam9x60ek_mmc_defconfig b/configs/sam9x60ek_mmc_defconfig
> index 268a485456..9dcceebc9b 100644
> --- a/configs/sam9x60ek_mmc_defconfig
> +++ b/configs/sam9x60ek_mmc_defconfig
> @@ -59,6 +59,7 @@ CONFIG_CLK_CCF=y
>  CONFIG_CLK_AT91=y
>  CONFIG_AT91_GENERIC_CLK=y
>  CONFIG_AT91_SAM9X60_PLL=y
> +CONFIG_AT91_SAM9X60_USB=y
>  CONFIG_CPU=y
>  CONFIG_AT91_GPIO=y
>  CONFIG_DM_I2C=y
> diff --git a/configs/sam9x60ek_nandflash_defconfig 
> b/configs/sam9x60ek_nandflash_defconfig
> index a9cbb6e953..86b3751072 100644
> --- a/configs/sam9x60ek_nandflash_defconfig
> +++ b/configs/sam9x60ek_nandflash_defconfig
> @@ -61,6 +61,7 @@ CONFIG_CLK_CCF=y
>  CONFIG_CLK_AT91=y
>  CONFIG_AT91_GENERIC_CLK=y
>  CONFIG_AT91_SAM9X60_PLL=y
> +CONFIG_AT91_SAM9X60_USB=y
>  CONFIG_CPU=y
>  CONFIG_AT91_GPIO=y
>  CONFIG_DM_I2C=y
> diff --git a/configs/sam9x60ek_qspiflash_defconfig 
> b/configs/sam9x60ek_qspiflash_defconfig
> index 72f08f1375..8b66dd22cd 100644
> --- a/configs/sam9x60ek_qspiflash_defconfig
> +++ b/configs/sam9x60ek_qspiflash_defconfig
> @@ -61,6 +61,7 @@ CONFIG_CLK_CCF=y
>  CONFIG_CLK_AT91=y
>  CONFIG_AT91_GENERIC_CLK=y
>  CONFIG_AT91_SAM9X60_PLL=y
> +CONFIG_AT91_SAM9X60_USB=y
>  CONFIG_CPU=y
>  CONFIG_AT91_GPIO=y
>  CONFIG_DM_I2C=y

Re: [PATCH 4/5] clk: at91: sam9x60: Add initial setup of UPLL and USBCK rates

[Claudiu.Beznea]

On 23.12.2022 14:33, Sergiu Moga wrote:
> In order for some of the functionalities, such as the USB clocks,
> to work properly we need some clocks to be properly initialised
> at the very beginning of booting.
> 
> Signed-off-by: Sergiu Moga 

Reviewed-by: Claudiu Beznea 


> ---
>  drivers/clk/at91/sam9x60.c | 30 ++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/clk/at91/sam9x60.c b/drivers/clk/at91/sam9x60.c
> index 14c2ffcac1..e2f72446d5 100644
> --- a/drivers/clk/at91/sam9x60.c
> +++ b/drivers/clk/at91/sam9x60.c
> @@ -378,6 +378,31 @@ static const struct {
>   { .n = "dbgu_gclk",   .id = 47, },
>  };
>  
> +/**
> + * Clock setup description
> + * @cid: clock id corresponding to clock subsystem
> + * @pid: parent clock id corresponding to clock subsystem
> + * @rate:clock rate
> + * @prate:   parent rate
> + */
> +static const struct pmc_clk_setup sam9x60_clk_setup[] = {
> + {
> + .cid = AT91_TO_CLK_ID(PMC_TYPE_CORE, ID_PLL_U_FRAC),
> + .rate = 96000,
> + },
> +
> + {
> + .cid = AT91_TO_CLK_ID(PMC_TYPE_CORE, ID_PLL_U_DIV),
> + .rate = 48000,
> + },
> +
> + {
> + .cid = AT91_TO_CLK_ID(PMC_TYPE_CORE, ID_USBCK),
> + .pid = AT91_TO_CLK_ID(PMC_TYPE_CORE, ID_PLL_U_DIV),
> + .rate = 4800,
> + },
> +};
> +
>  #define prepare_mux_table(_allocs, _index, _dst, _src, _num, _label) \
>   do {\
>   int _i; \
> @@ -668,6 +693,11 @@ static int sam9x60_clk_probe(struct udevice *dev)
>   clk_dm(AT91_TO_CLK_ID(PMC_TYPE_GCK, sam9x60_gck[i].id), c);
>   }
>  
> + /* Setup clocks. */
> + ret = at91_clk_setup(sam9x60_clk_setup, ARRAY_SIZE(sam9x60_clk_setup));
> + if (ret)
> + goto fail;
> +
>   return 0;
>  
>  fail:

Re: [PATCH 2/5] clk: at91: sam9x60: Register the required clocks for USB

[Claudiu.Beznea]

On 23.12.2022 14:33, Sergiu Moga wrote:
> Register into DM the clocks required to properly enable USB functionality
> within the bootloader.
> 
> Signed-off-by: Sergiu Moga 

Reviewed-by: Claudiu Beznea 


> ---
>  drivers/clk/at91/sam9x60.c | 33 +
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/clk/at91/sam9x60.c b/drivers/clk/at91/sam9x60.c
> index 6b5486c6c9..14c2ffcac1 100644
> --- a/drivers/clk/at91/sam9x60.c
> +++ b/drivers/clk/at91/sam9x60.c
> @@ -76,6 +76,8 @@ enum pmc_clk_ids {
>   ID_QSPI = 18,
>  
>   ID_MCK_PRES = 19,
> + ID_USBCK= 20,
> + ID_UHPCK= 21,
>  
>   ID_MAX,
>  };
> @@ -99,6 +101,7 @@ static const char *clk_names[] = {
>   [ID_PLL_A_DIV]  = "plla_divpmcck",
>   [ID_MCK_PRES]   = "mck_pres",
>   [ID_MCK_DIV]= "mck_div",
> + [ID_USBCK]  = "usbck",
>  };
>  
>  /* Fractional PLL output range. */
> @@ -171,6 +174,13 @@ static const struct clk_pcr_layout pcr_layout = {
>   .pid_mask = GENMASK(6, 0),
>  };
>  
> +/* USB clock layout */
> +static const struct clk_usbck_layout usbck_layout = {
> + .offset = 0x38,
> + .usbs_mask = GENMASK(1, 0),
> + .usbdiv_mask = GENMASK(11, 8),
> +};
> +
>  /**
>   * PLL clocks description
>   * @n:   clock name
> @@ -266,6 +276,7 @@ static const struct {
>   u8 cid;
>  } sam9x60_systemck[] = {
>   { .n = "ddrck", .p = "mck_div",  .id = 2, .cid = ID_DDR, },
> + { .n = "uhpck", .p = "usbck",.id = 6, .cid = ID_UHPCK },
>   { .n = "pck0",  .p = "prog0",.id = 8, .cid = ID_PCK0, },
>   { .n = "pck1",  .p = "prog1",.id = 9, .cid = ID_PCK1, },
>   { .n = "qspick",.p = "mck_div",  .id = 19, .cid = ID_QSPI, },
> @@ -543,6 +554,28 @@ static int sam9x60_clk_probe(struct udevice *dev)
>   }
>   clk_dm(AT91_TO_CLK_ID(PMC_TYPE_CORE, ID_MCK_DIV), c);
>  
> + /* Register usbck. */
> + p[0] = clk_names[ID_PLL_A_DIV];
> + p[1] = clk_names[ID_PLL_U_DIV];
> + p[2] = clk_names[ID_MAIN_XTAL];
> + m[0] = 0;
> + m[1] = 1;
> + m[2] = 2;
> + cm[0] = AT91_TO_CLK_ID(PMC_TYPE_CORE, ID_PLL_A_DIV);
> + cm[1] = AT91_TO_CLK_ID(PMC_TYPE_CORE, ID_PLL_U_DIV);
> + cm[2] = AT91_TO_CLK_ID(PMC_TYPE_CORE, ID_MAIN_XTAL);
> + prepare_mux_table(clkmuxallocs, clkmuxallocindex, tmpclkmux, cm,
> +   3, fail);
> + prepare_mux_table(muxallocs, muxallocindex, tmpmux, m, 3, fail);
> + c = sam9x60_clk_register_usb(base, clk_names[ID_USBCK], p, 3,
> +  _layout, tmpclkmux, tmpmux,
> +  ID_USBCK);
> + if (IS_ERR(c)) {
> + ret = PTR_ERR(c);
> + goto fail;
> + }
> + clk_dm(AT91_TO_CLK_ID(PMC_TYPE_CORE, ID_USBCK), c);
> +
>   /* Register programmable clocks. */
>   p[0] = clk_names[ID_MD_SLCK];
>   p[1] = clk_names[ID_TD_SLCK];

Re: [PATCH 1/5] clk: at91: Add support for sam9x60 USB clock

[Claudiu.Beznea]

On 23.12.2022 14:33, Sergiu Moga wrote:
> Implement sam9x60 USB clock driver. This clock has
> three parents: PLLA, UPLL and MAINXTAL. The driver is
> aware of the three possible parents with the help of the
> two mux tables provied to the driver during the registration
> of the clock.
> 
> Signed-off-by: Sergiu Moga 

Reviewed-by: Claudiu Beznea 



> ---
>  drivers/clk/at91/Kconfig   |   7 ++
>  drivers/clk/at91/Makefile  |   1 +
>  drivers/clk/at91/clk-sam9x60-usb.c | 157 +
>  drivers/clk/at91/pmc.h |  11 ++
>  4 files changed, 176 insertions(+)
>  create mode 100644 drivers/clk/at91/clk-sam9x60-usb.c
> 
> diff --git a/drivers/clk/at91/Kconfig b/drivers/clk/at91/Kconfig
> index 4abc8026b4..4563892647 100644
> --- a/drivers/clk/at91/Kconfig
> +++ b/drivers/clk/at91/Kconfig
> @@ -61,3 +61,10 @@ config AT91_SAM9X60_PLL
>   help
> This option is used to enable the AT91 SAM9X60's PLL clock
> driver.
> +
> +config AT91_SAM9X60_USB
> + bool "USB Clock support for SAM9X60 SoCs"
> + depends on CLK_AT91
> + help
> +   This option is used to enable the AT91 SAM9X60's USB clock
> +   driver.
> diff --git a/drivers/clk/at91/Makefile b/drivers/clk/at91/Makefile
> index 580b406d7b..e53dcb4ca7 100644
> --- a/drivers/clk/at91/Makefile
> +++ b/drivers/clk/at91/Makefile
> @@ -9,6 +9,7 @@ obj-y += clk-peripheral.o
>  obj-$(CONFIG_AT91_GENERIC_CLK)   += clk-generic.o
>  obj-$(CONFIG_AT91_UTMI)  += clk-utmi.o
>  obj-$(CONFIG_AT91_SAM9X60_PLL)   += clk-sam9x60-pll.o
> +obj-$(CONFIG_AT91_SAM9X60_USB)   += clk-sam9x60-usb.o
>  obj-$(CONFIG_SAMA7G5)+= sama7g5.o
>  obj-$(CONFIG_SAM9X60)+= sam9x60.o
>  else
> diff --git a/drivers/clk/at91/clk-sam9x60-usb.c 
> b/drivers/clk/at91/clk-sam9x60-usb.c
> new file mode 100644
> index 00..798fa9eb3c
> --- /dev/null
> +++ b/drivers/clk/at91/clk-sam9x60-usb.c
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * SAM9X60's USB Clock support.
> + *
> + * Copyright (C) 2022 Microchip Technology Inc. and its subsidiaries
> + *
> + * Author: Sergiu Moga 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "pmc.h"
> +
> +#define UBOOT_DM_CLK_AT91_SAM9X60_USB"at91-sam9x60-usb-clk"
> +
> +struct sam9x60_usb {
> + const struct clk_usbck_layout   *layout;
> + void__iomem *base;
> + struct clk  clk;
> + const u32   *clk_mux_table;
> + const u32   *mux_table;
> + const char * const  *parent_names;
> + u32 num_parents;
> + u8  id;
> +};
> +
> +#define to_sam9x60_usb(_clk) container_of(_clk, struct sam9x60_usb, clk)
> +#define USB_MAX_DIV  15
> +
> +static int sam9x60_usb_clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> + struct sam9x60_usb *usb = to_sam9x60_usb(clk);
> + int index;
> + u32 val;
> +
> + index = at91_clk_mux_val_to_index(usb->clk_mux_table, usb->num_parents,
> +   parent->id);
> + if (index < 0)
> + return index;
> +
> + index = at91_clk_mux_index_to_val(usb->mux_table, usb->num_parents,
> +   index);
> + if (index < 0)
> + return index;
> +
> + pmc_read(usb->base, usb->layout->offset, );
> + val &= ~usb->layout->usbs_mask;
> + val |= index << (ffs(usb->layout->usbs_mask - 1));
> + pmc_write(usb->base, usb->layout->offset, val);
> +
> + return 0;
> +}
> +
> +static ulong sam9x60_usb_clk_get_rate(struct clk *clk)
> +{
> + struct sam9x60_usb *usb = to_sam9x60_usb(clk);
> + ulong parent_rate = clk_get_parent_rate(clk);
> + u32 val, usbdiv;
> +
> + if (!parent_rate)
> + return 0;
> +
> + pmc_read(usb->base, usb->layout->offset, );
> + usbdiv = (val & usb->layout->usbdiv_mask) >>
> + (ffs(usb->layout->usbdiv_mask) - 1);
> + return parent_rate / (usbdiv + 1);
> +}
> +
> +static ulong sam9x60_usb_clk_set_rate(struct clk *clk, ulong rate)
> +{
> + struct sam9x60_usb *usb = to_sam9x60_usb(clk);
> + ulong parent_rate = clk_get_parent_rate(clk);
> + u32 usbdiv, val;
> +
> + if (!parent_rate)
> + return 0;
> +
> + usbdiv = DIV_ROUND_CLOSEST(parent_rate, rate);
> + if (usbdiv > USB_MAX_DIV + 1 || !usbdiv)
> + return 0;
> +
> + pmc_read(usb->base, usb->layout->offset, );
> + val &= usb->layout->usbdiv_mask;
> + val |= (usbdiv - 1) << (ffs(usb->layout->usbdiv_mask) - 1);
> + pmc_write(usb->base, usb->layout->offset, val);
> +
> + return parent_rate / usbdiv;
> +}
> +
> +static const struct clk_ops sam9x60_usb_ops = {
> + .set_parent = sam9x60_usb_clk_set_parent,
> + 

Re: [PATCH v3 12/19] phy: at91: Add support for the USB 2.0 PHY's of SAMA7

[Claudiu.Beznea]

On 12.12.2022 15:39, Sergiu Moga wrote:
> In order to have USB functionality, drivers for SAMA7's
> USB 2.0 PHY's have been added. There is one driver
> for UTMI clock's SFR and RESET required functionalities and
> one for its three possible subclocks of the phy's themselves.
> In order for this layout to properly work in conjunction with
> CCF and DT, the former driver will also act as a clock provider
> for the three phy's with the help of a custom hook into the
> driver's of_xlate method.
> 
> Signed-off-by: Sergiu Moga 
> Tested-by: Mihai Sain 
> ---
> 
> v1 -> v3:
> - No change
> 
> 
>  drivers/phy/Kconfig  |  10 ++
>  drivers/phy/Makefile |   1 +
>  drivers/phy/phy-sama7-usb.c  |  92 ++
>  drivers/phy/phy-sama7-utmi-clk.c | 202 +++
>  4 files changed, 305 insertions(+)
>  create mode 100644 drivers/phy/phy-sama7-usb.c
>  create mode 100644 drivers/phy/phy-sama7-utmi-clk.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index cf4d5908d7..9fbb956783 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -281,6 +281,16 @@ config PHY_XILINX_ZYNQMP
> Enable this to support ZynqMP High Speed Gigabit Transceiver
> that is part of ZynqMP SoC.
>  
> +config PHY_MICROCHIP_SAMA7_USB
> +   tristate "Microchip SAMA7 USB 2.0 PHY"
> +   depends on PHY && ARCH_AT91
> +   help
> +  Enable this to support SAMA7 USB 2.0 PHY.
> +
> +  The USB 2.0 PHY integrates high-speed, full-speed and low-speed
> +  termination and signal switching. With a single resistor, it
> +  requires minimal external components.
> +
>  source "drivers/phy/rockchip/Kconfig"
>  source "drivers/phy/cadence/Kconfig"
>  source "drivers/phy/ti/Kconfig"
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index a3b9f3c5b1..9d50affd47 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
>  obj-$(CONFIG_PHY_NPCM_USB) += phy-npcm-usb.o
>  obj-$(CONFIG_PHY_IMX8MQ_USB) += phy-imx8mq-usb.o
>  obj-$(CONFIG_PHY_XILINX_ZYNQMP) += phy-zynqmp.o
> +obj-$(CONFIG_PHY_MICROCHIP_SAMA7_USB)  += phy-sama7-utmi-clk.o 
> phy-sama7-usb.o
>  obj-y += cadence/
>  obj-y += ti/
>  obj-y += qcom/
> diff --git a/drivers/phy/phy-sama7-usb.c b/drivers/phy/phy-sama7-usb.c
> new file mode 100644
> index 00..b6fe40ecc1
> --- /dev/null
> +++ b/drivers/phy/phy-sama7-usb.c
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Support for Atmel/Microchip USB PHY's.
> + *
> + * Copyright (C) 2022 Microchip Technology Inc. and its subsidiaries
> + *
> + * Author: Sergiu Moga 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct sama7_usb_phy {
> + struct clk *uclk;
> + struct regmap *sfr;
> + int port;
> +};
> +
> +int sama7_usb_phy_init(struct phy *phy)
> +{
> + struct sama7_usb_phy *sama7_phy = dev_get_priv(phy->dev);
> + int port = sama7_phy->port;
> +
> + regmap_update_bits(sama7_phy->sfr, SAMA7_SFR_UTMI0R(port),
> +SAMA7_SFR_UTMI_RX_TX_PREEM_AMP_TUNE_1X,
> +SAMA7_SFR_UTMI_RX_TX_PREEM_AMP_TUNE_1X);
> +
> + regmap_update_bits(sama7_phy->sfr, SAMA7_SFR_UTMI0R(port),
> +SAMA7_SFR_UTMI_RX_VBUS,
> +SAMA7_SFR_UTMI_RX_VBUS);
> +
> + return 0;
> +}
> +
> +int sama7_phy_power_on(struct phy *phy)
> +{
> + struct sama7_usb_phy *sama7_phy = dev_get_priv(phy->dev);
> +
> + clk_prepare_enable(sama7_phy->uclk);
> +
> + return 0;
> +}
> +
> +int sama7_phy_power_off(struct phy *phy)
> +{
> + struct sama7_usb_phy *sama7_phy = dev_get_priv(phy->dev);
> +
> + clk_disable_unprepare(sama7_phy->uclk);
> +
> + return 0;
> +}
> +
> +int sama7_usb_phy_probe(struct udevice *dev)
> +{
> + struct sama7_usb_phy *sama7_phy = dev_get_priv(dev);
> +
> + sama7_phy->uclk = devm_clk_get(dev, "utmi_clk");
> + if (IS_ERR(sama7_phy->uclk))
> + return PTR_ERR(sama7_phy->uclk);
> +
> + sama7_phy->sfr = syscon_regmap_lookup_by_phandle(dev, "sfr-phandle");
> + if (IS_ERR(sama7_phy->sfr)) {
> + sama7_phy->sfr = NULL;

Is this needed?

> + return PTR_ERR(sama7_phy->sfr);
> + }
> +
> + return dev_read_u32(dev, "reg", _phy->port);
> +}
> +
> +static const struct phy_ops sama7_usb_phy_ops = {
> + .init = sama7_usb_phy_init,
> + .power_on = sama7_phy_power_on,
> + .power_off = sama7_phy_power_off,
> +};
> +
> +static const struct udevice_id sama7_usb_phy_of_match[] = {
> + { .compatible = "microchip,sama7g5-usb-phy", },
> + { },
> +};
> +
> +U_BOOT_DRIVER(sama7_usb_phy_driver) = {
> + .name = "sama7-usb-phy",
> + .id = UCLASS_PHY,
> + .of_match = sama7_usb_phy_of_match,
> + .ops = _usb_phy_ops,
> + .probe = sama7_usb_phy_probe,
> + .priv_auto = sizeof(struct sama7_usb_phy),
> +};
> 

Re: [PATCH v2 12/19] reset: at91: Add reset driver for basic assert/deassert operations

[Claudiu.Beznea]

On 08.12.2022 11:47, Sergiu Moga wrote:
> Add support for at91 reset controller's basic assert/deassert
> operations. Since this driver conflicts with the
> SYSRESET driver because they both bind to the same RSTC node,
> implement a custom bind hook that would manually bind the
> sysreset driver, if enabled, to the same RSTC DT node.
> Furthermore, delete the no longer needed compatibles from the
> SYSRESET driver and rename it to make sure than any possible
> conflicts are avoided.
> 
> Signed-off-by: Sergiu Moga 
> Tested-by: Mihai Sain 

Reviewed-by: Claudiu Beznea 


> ---
> 
> v1 -> v2:
> - No change
> 
>  drivers/reset/Kconfig|   8 ++
>  drivers/reset/Makefile   |   1 +
>  drivers/reset/reset-at91.c   | 141 +++
>  drivers/sysreset/sysreset_at91.c |  10 +--
>  4 files changed, 151 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/reset/reset-at91.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 4cb0ba0850..e4039d7474 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -211,4 +211,12 @@ config RESET_DRA7
>   help
> Support for TI DRA7-RESET subsystem. Basic Assert/Deassert
> is supported.
> +
> +config RESET_AT91
> + bool "Enable support for Microchip/Atmel Reset Controller driver"
> + depends on DM_RESET && ARCH_AT91
> + help
> +   This enables the Reset Controller driver support for Microchip/Atmel
> +   SoCs. Mainly used to expose assert/deassert methods to other drivers
> +   that require it.
>  endmenu
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 0620b62809..6c8b45ecba 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -31,3 +31,4 @@ obj-$(CONFIG_RESET_RASPBERRYPI) += reset-raspberrypi.o
>  obj-$(CONFIG_RESET_SCMI) += reset-scmi.o
>  obj-$(CONFIG_RESET_ZYNQMP) += reset-zynqmp.o
>  obj-$(CONFIG_RESET_DRA7) += reset-dra7.o
> +obj-$(CONFIG_RESET_AT91) += reset-at91.o
> diff --git a/drivers/reset/reset-at91.c b/drivers/reset/reset-at91.c
> new file mode 100644
> index 00..165c87acdc
> --- /dev/null
> +++ b/drivers/reset/reset-at91.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Support for Atmel/Microchip Reset Controller.
> + *
> + * Copyright (C) 2022 Microchip Technology Inc. and its subsidiaries
> + *
> + * Author: Sergiu Moga 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct at91_reset {
> + void __iomem *dev_base;
> + struct at91_reset_data *data;
> +};
> +
> +struct at91_reset_data {
> + u32 n_device_reset;
> + u8 device_reset_min_id;
> + u8 device_reset_max_id;
> +};
> +
> +static const struct at91_reset_data sama7g5_data = {
> + .n_device_reset = 3,
> + .device_reset_min_id = SAMA7G5_RESET_USB_PHY1,
> + .device_reset_max_id = SAMA7G5_RESET_USB_PHY3,
> +};
> +
> +static int at91_rst_update(struct at91_reset *reset, unsigned long id,
> +bool assert)
> +{
> + u32 val;
> +
> + if (!reset->dev_base)
> + return 0;
> +
> + val = readl(reset->dev_base);
> + if (assert)
> + val |= BIT(id);
> + else
> + val &= ~BIT(id);
> + writel(val, reset->dev_base);
> +
> + return 0;
> +}
> +
> +static int at91_reset_of_xlate(struct reset_ctl *reset_ctl,
> +struct ofnode_phandle_args *args)
> +{
> + struct at91_reset *reset = dev_get_priv(reset_ctl->dev);
> +
> + if (!reset->data->n_device_reset ||
> + args->args[0] < reset->data->device_reset_min_id ||
> + args->args[0] > reset->data->device_reset_max_id)
> + return -EINVAL;
> +
> + reset_ctl->id = args->args[0];
> +
> + return 0;
> +}
> +
> +static int at91_rst_assert(struct reset_ctl *reset_ctl)
> +{
> + struct at91_reset *reset = dev_get_priv(reset_ctl->dev);
> +
> + return at91_rst_update(reset, reset_ctl->id, true);
> +}
> +
> +static int at91_rst_deassert(struct reset_ctl *reset_ctl)
> +{
> + struct at91_reset *reset = dev_get_priv(reset_ctl->dev);
> +
> + return at91_rst_update(reset, reset_ctl->id, false);
> +}
> +
> +struct reset_ops at91_reset_ops = {
> + .of_xlate = at91_reset_of_xlate,
> + .rst_assert = at91_rst_assert,
> + .rst_deassert = at91_rst_deassert,
> +};
> +
> +static int at91_reset_probe(struct udevice *dev)
> +{
> + struct at91_reset *reset = dev_get_priv(dev);
> + struct clk sclk;
> + int ret;
> +
> + reset->data = (struct at91_reset_data *)dev_get_driver_data(dev);
> + reset->dev_base = dev_remap_addr_index(dev, 1);
> + if (reset->data && reset->data->n_device_reset && !reset->dev_base)
> + return -EINVAL;
> +
> + ret = clk_get_by_index(dev, 0, );
> + if (ret)
> + return ret;
> +
> + return clk_prepare_enable();
> +}
> +
> +static int at91_reset_bind(struct udevice 

Re: [PATCH v2 02/19] clk: at91: Add support for sam9x60 USB clock

[Claudiu.Beznea]

On 08.12.2022 11:47, Sergiu Moga wrote:
> Implement sam9x60 USB clock driver. This clock has
> three parents: PLLA, UPLL and MAINXTAL. The driver is
> aware of the three possible parents with the help of the
> two mux tables provied to the driver during the registration
> of the clock.
> 
> Signed-off-by: Sergiu Moga 
> ---
> 
> 
> 
> v1 -> v2:
> - No change
> 
> 
> 
>  drivers/clk/at91/Kconfig   |   7 ++
>  drivers/clk/at91/Makefile  |   1 +
>  drivers/clk/at91/clk-sam9x60-usb.c | 156 +
>  drivers/clk/at91/pmc.h |  11 ++
>  4 files changed, 175 insertions(+)
>  create mode 100644 drivers/clk/at91/clk-sam9x60-usb.c
> 
> diff --git a/drivers/clk/at91/Kconfig b/drivers/clk/at91/Kconfig
> index 4abc8026b4..4563892647 100644
> --- a/drivers/clk/at91/Kconfig
> +++ b/drivers/clk/at91/Kconfig
> @@ -61,3 +61,10 @@ config AT91_SAM9X60_PLL
>   help
> This option is used to enable the AT91 SAM9X60's PLL clock
> driver.
> +
> +config AT91_SAM9X60_USB
> + bool "USB Clock support for SAM9X60 SoCs"
> + depends on CLK_AT91
> + help
> +   This option is used to enable the AT91 SAM9X60's USB clock
> +   driver.
> diff --git a/drivers/clk/at91/Makefile b/drivers/clk/at91/Makefile
> index 580b406d7b..e53dcb4ca7 100644
> --- a/drivers/clk/at91/Makefile
> +++ b/drivers/clk/at91/Makefile
> @@ -9,6 +9,7 @@ obj-y += clk-peripheral.o
>  obj-$(CONFIG_AT91_GENERIC_CLK)   += clk-generic.o
>  obj-$(CONFIG_AT91_UTMI)  += clk-utmi.o
>  obj-$(CONFIG_AT91_SAM9X60_PLL)   += clk-sam9x60-pll.o
> +obj-$(CONFIG_AT91_SAM9X60_USB)   += clk-sam9x60-usb.o
>  obj-$(CONFIG_SAMA7G5)+= sama7g5.o
>  obj-$(CONFIG_SAM9X60)+= sam9x60.o
>  else
> diff --git a/drivers/clk/at91/clk-sam9x60-usb.c 
> b/drivers/clk/at91/clk-sam9x60-usb.c
> new file mode 100644
> index 00..0ccdc1494d
> --- /dev/null
> +++ b/drivers/clk/at91/clk-sam9x60-usb.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * SAM9X60's USB Clock support.
> + *
> + * Copyright (C) 2022 Microchip Technology Inc. and its subsidiaries
> + *
> + * Author: Sergiu Moga 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "pmc.h"
> +
> +#define UBOOT_DM_CLK_AT91_SAM9X60_USB"at91-sam9x60-usb-clk"
> +
> +struct sam9x60_usb {
> + const struct clk_usbck_layout   *layout;
> + void__iomem *base;
> + struct clk  clk;
> + const u32   *clk_mux_table;
> + const u32   *mux_table;
> + const char * const  *parent_names;
> + u32 num_parents;
> + u8  id;
> +};
> +
> +#define to_sam9x60_usb(_clk) container_of(_clk, struct sam9x60_usb, clk)
> +#define USB_MAX_DIV  15
> +
> +static int sam9x60_usb_clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> + struct sam9x60_usb *usb = to_sam9x60_usb(clk);
> + u32 val, index;
> +
> + index = at91_clk_mux_val_to_index(usb->clk_mux_table, usb->num_parents,
> +   parent->id);
> + if (index < 0)
> + return index;
> +
> + index = at91_clk_mux_index_to_val(usb->mux_table, usb->num_parents,
> +   index);
> + if (index < 0)
> + return index;
> +
> + pmc_read(usb->base, usb->layout->offset, );
> + val &= ~usb->layout->usbs_mask;
> + val |= index << (ffs(usb->layout->usbs_mask - 1));

You can use FIELD_PREP() here.

> + pmc_write(usb->base, usb->layout->offset, val);
> +
> + return 0;
> +}
> +
> +static ulong sam9x60_usb_clk_get_rate(struct clk *clk)
> +{
> + struct sam9x60_usb *usb = to_sam9x60_usb(clk);
> + ulong parent_rate = clk_get_parent_rate(clk);
> + u32 val, usbdiv;
> +
> + if (!parent_rate)
> + return 0;
> +
> + pmc_read(usb->base, usb->layout->offset, );
> + usbdiv = (val & usb->layout->usbdiv_mask) >>
> + (ffs(usb->layout->usbdiv_mask) - 1);

And FIELD_GET() here.

> + return parent_rate / (usbdiv + 1);
> +}
> +
> +static ulong sam9x60_usb_clk_set_rate(struct clk *clk, ulong rate)
> +{
> + struct sam9x60_usb *usb = to_sam9x60_usb(clk);
> + ulong parent_rate = clk_get_parent_rate(clk);
> + u32 usbdiv, val;
> +
> + if (!parent_rate)
> + return 0;
> +
> + usbdiv = DIV_ROUND_CLOSEST(parent_rate, rate);
> + if (usbdiv > USB_MAX_DIV + 1 || !usbdiv)
> + return 0;
> +
> + pmc_read(usb->base, usb->layout->offset, );
> + val &= usb->layout->usbdiv_mask;
> + val |= (usbdiv - 1) << (ffs(usb->layout->usbdiv_mask) - 1);

FIELD_PREP()

> + pmc_write(usb->base, usb->layout->offset, val);
> +
> + return parent_rate / usbdiv;
> +}
> +
> +static const struct clk_ops 

Re: [PATCH] ARM: dts: at91: sama5d2: fix wrong interrupt-cells property

[Claudiu.Beznea]

On 12.12.2022 11:59, Eugen Hristev wrote:
> The PMC node is not an interrupt provider, so it must not have
> interrupt-cells.
> 
> This fixes the warning (on newer DTC):
> arch/arm/dts/sama5d2.dtsi:82.22-602.6: Warning (interrupt_provider): 
> /ahb/apb/pmc@f0014000: '#interrupt-cells' found, but node is not an interrupt 
> provider
> 
> Fixes: 2c4b2dd289 ("ARM: at91/dt: Add device tree for SAMA5D2 Xplained")
> Signed-off-by: Eugen Hristev 

Reviewed-by: Claudiu Beznea 


> ---
>  arch/arm/dts/sama5d2.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/dts/sama5d2.dtsi b/arch/arm/dts/sama5d2.dtsi
> index 790b746ed1..187c2ff2fb 100644
> --- a/arch/arm/dts/sama5d2.dtsi
> +++ b/arch/arm/dts/sama5d2.dtsi
> @@ -84,7 +84,6 @@
>   reg = <0xf0014000 0x160>;
>   #address-cells = <1>;
>   #size-cells = <0>;
> - #interrupt-cells = <1>;
>   u-boot,dm-pre-reloc;
>  
>   main: mainck {

Re: [PATCH 1/3] dt-bindings: mfd: add at91-usart.h from Linux

[Claudiu.Beznea]

On 25.11.2022 09:54, Eugen Hristev wrote:
> Copy include file dt-bindings/mfd/at91-usart.h from Linux
> 
> Signed-off-by: Eugen Hristev 

Reviewed-by: Claudiu Beznea 


> ---
>  include/dt-bindings/mfd/at91-usart.h | 17 +
>  1 file changed, 17 insertions(+)
>  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> 
> diff --git a/include/dt-bindings/mfd/at91-usart.h 
> b/include/dt-bindings/mfd/at91-usart.h
> new file mode 100644
> index 00..2de5bc312e
> --- /dev/null
> +++ b/include/dt-bindings/mfd/at91-usart.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * This header provides macros for AT91 USART DT bindings.
> + *
> + * Copyright (C) 2018 Microchip Technology
> + *
> + * Author: Radu Pirea 
> + *
> + */
> +
> +#ifndef __DT_BINDINGS_AT91_USART_H__
> +#define __DT_BINDINGS_AT91_USART_H__
> +
> +#define AT91_USART_MODE_SERIAL   0
> +#define AT91_USART_MODE_SPI  1
> +
> +#endif /* __DT_BINDINGS_AT91_USART_H__ */

Re: [PATCH 2/3] sysreset: at91: add compatible with microchip,sama7g5-rstc

[Claudiu.Beznea]

On 25.11.2022 09:54, Eugen Hristev wrote:
> As documented in bindings doc in kernel 6.0:
> https://elixir.bootlin.com/linux/v6.0/source/Documentation/devicetree/bindings/reset/atmel,at91sam9260-reset.yaml
> 
> Signed-off-by: Eugen Hristev 

Reviewed-by: Claudiu Beznea 


> ---
>  drivers/sysreset/sysreset_at91.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/sysreset/sysreset_at91.c 
> b/drivers/sysreset/sysreset_at91.c
> index 24b87ee987..6119a29927 100644
> --- a/drivers/sysreset/sysreset_at91.c
> +++ b/drivers/sysreset/sysreset_at91.c
> @@ -59,6 +59,7 @@ static struct sysreset_ops at91_sysreset = {
>  static const struct udevice_id a91_sysreset_ids[] = {
>   { .compatible = "atmel,sama5d3-rstc" },
>   { .compatible = "microchip,sam9x60-rstc" },
> + { .compatible = "microchip,sama7g5-rstc" },
>   { }
>  };
>  

Re: [PATCH 3/3] ARM: dts: at91: sama7g5/sama7g5ek: align DT with kernel 6.1

[Claudiu.Beznea]

On 25.11.2022 09:54, Eugen Hristev wrote:
> Align the DT with current Linux 6.1 tree, wherever possible.
> 
> Signed-off-by: Eugen Hristev 

Reviewed-by: Claudiu Beznea 


> ---
>  arch/arm/dts/at91-sama7g5ek.dts | 23 +++
>  arch/arm/dts/sama7g5.dtsi   | 27 ---
>  2 files changed, 27 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/arm/dts/at91-sama7g5ek.dts b/arch/arm/dts/at91-sama7g5ek.dts
> index aed84f15a1..9b247fcaf6 100644
> --- a/arch/arm/dts/at91-sama7g5ek.dts
> +++ b/arch/arm/dts/at91-sama7g5ek.dts
> @@ -45,13 +45,13 @@
>   };
>   };
>  
> - gpio_keys {
> + gpio-keys {
>   compatible = "gpio-keys";
>  
>   pinctrl-names = "default";
>   pinctrl-0 = <_key_gpio_default>;
>  
> - bp1 {
> + button {
>   label = "PB_USER";
>   gpios = < PIN_PA12 GPIO_ACTIVE_LOW>;
>   linux,code = ;
> @@ -244,8 +244,8 @@
>   regulators {
>   vdd_3v3: VDD_IO {
>   regulator-name = "VDD_IO";
> - regulator-min-microvolt = <120>;
> - regulator-max-microvolt = <370>;
> + regulator-min-microvolt = <330>;
> + regulator-max-microvolt = <330>;
>   regulator-initial-mode = <2>;
>   regulator-allowed-modes = <2>, <4>;
>   regulator-always-on;
> @@ -264,8 +264,8 @@
>  
>   vddioddr: VDD_DDR {
>   regulator-name = "VDD_DDR";
> - regulator-min-microvolt = <130>;
> - regulator-max-microvolt = <145>;
> + regulator-min-microvolt = <135>;
> + regulator-max-microvolt = <135>;
>   regulator-initial-mode = <2>;
>   regulator-allowed-modes = <2>, <4>;
>   regulator-always-on;
> @@ -285,8 +285,8 @@
>  
>   vddcore: VDD_CORE {
>   regulator-name = "VDD_CORE";
> - regulator-min-microvolt = <110>;
> - regulator-max-microvolt = <185>;
> + regulator-min-microvolt = <115>;
> + regulator-max-microvolt = <115>;
>   regulator-initial-mode = <2>;
>   regulator-allowed-modes = <2>, <4>;
>   regulator-always-on;
> @@ -306,7 +306,7 @@
>   vddcpu: VDD_OTHER {
>   regulator-name = "VDD_OTHER";
>   regulator-min-microvolt = <105>;
> - regulator-max-microvolt = <185>;
> + regulator-max-microvolt = <125>;
>   regulator-initial-mode = <2>;
>   regulator-allowed-modes = <2>, <4>;
>   regulator-ramp-delay = <3125>;
> @@ -326,8 +326,8 @@
>  
>   vldo1: LDO1 {
>   regulator-name = "LDO1";
> - regulator-min-microvolt = <120>;
> - regulator-max-microvolt = <370>;
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
>   regulator-always-on;
>  
>   regulator-state-standby {
> @@ -707,7 +707,6 @@
>   ck_cd_rstn_vddsel {
>   pinmux = ,
>,
> -  ,
>;
>   slew-rate = <0>;
>   bias-pull-up;
> diff --git a/arch/arm/dts/sama7g5.dtsi b/arch/arm/dts/sama7g5.dtsi
> index d38090d7dd..6388a60e53 100644
> --- a/arch/arm/dts/sama7g5.dtsi
> +++ b/arch/arm/dts/sama7g5.dtsi
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  / {
>   model = "Microchip SAMA7G5 family SoC";
> @@ -195,11 +196,11 @@
>   ,
>   ,
>   ;
> - clocks = < PMC_TYPE_PERIPHERAL 11>;
>   interrupt-controller;
>   

Re: [PATCH] clk: at91: sam9x60: change parent clock from mck_pres to mck_div

[Claudiu.Beznea]

On 17.08.2022 10:04, Eugen Hristev wrote:
> From: Mihai Sain 
> 
> ddrck and qspick should have mck_div as parent clocks to be in sync with 
> linux driver.
> 
> Signed-off-by: Mihai Sain 

Reviewed-by: Claudiu Beznea 


> ---
>  drivers/clk/at91/sam9x60.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/at91/sam9x60.c b/drivers/clk/at91/sam9x60.c
> index 4d00ee2ddc..6b5486c6c9 100644
> --- a/drivers/clk/at91/sam9x60.c
> +++ b/drivers/clk/at91/sam9x60.c
> @@ -265,10 +265,10 @@ static const struct {
>   u8 id;
>   u8 cid;
>  } sam9x60_systemck[] = {
> - { .n = "ddrck", .p = "mck_pres", .id = 2, .cid = ID_DDR, },
> + { .n = "ddrck", .p = "mck_div",  .id = 2, .cid = ID_DDR, },
>   { .n = "pck0",  .p = "prog0",.id = 8, .cid = ID_PCK0, },
>   { .n = "pck1",  .p = "prog1",.id = 9, .cid = ID_PCK1, },
> - { .n = "qspick",.p = "mck_pres", .id = 19, .cid = ID_QSPI, },
> + { .n = "qspick",.p = "mck_div",  .id = 19, .cid = ID_QSPI, },
>  };
>  
>  /**

Re: SAMA5D3 Xplained: SPL broken after panic added to /lib/time.c:94

[Claudiu.Beznea]

On 23.03.2021 18:54, Manuel Luís Reis wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> Hi again,
> 
>> There are timers on the board. How come it used to work, before the
>> commit that breaks it ?
>>
>> I understand that nobody registers a driver in the UCLASS_TIMER , but
>> why was this enforced? and if this enforcement breaks our board, we can
>> either:
>> 1/ stop the enforcement
>> 2/ comply
>>
>> Either way, I think we have to see how we can get one timer to register
>> itself as an UCLASS_TIMER
> 
> I understand.
> 
>> This timer is a new hardware timer which is not present in the sama5d3 SoC.
>>
>> Sama5d3 should have old PIT timers, (programmable interrupt timer)
>> and most likely that code has not been converted to DM (UCLASSes)
>>
>> One possible approach is to convert specific old PIT code to comply with
>> DM requirements (or if that code is already available, we have to
>> use/enable it ).
>>
>> I add Claudiu in the mail thread, as he is more familiar with PIT and
>> timers than I am, and maybe he has some opinion related to this issue.
> 
> Thanks for clarifying this for me.
> 
> Look forward to hearing from Claudiu.

As it seems from the dump of dm_dump_all() the atmel_pit_timer is not
probed. I did a bit of debug and the dm_timer_init() ->
uclass_first_device() -> uclass_find_first_device() found zero timers
registered for UCLASS_TIMER. The driver is compiled.  Also checked that
atmel_pit_timer probe function is not called at all. The question should be
why it is not probed at all?

> 
> Cheers,
> 
> 
> 
> 
> On Tue, 23 Mar 2021 at 16:26,  wrote:
>>
>> On 3/23/21 6:08 PM, Manuel Luís Reis wrote:
>>> Hi again,
>>>
>>> FYI: As a small test I commented out the change you mentioned but got
>>> the same mistake. Begs to wonder if it is
>>>   related to the issue at hand.
>>>
>>> Going back to
>>> http://u-boot.10912.n7.nabble.com/PATCH-v2-time-Fix-get-ticks-being-non-monotonic-td426172.html
>>>
>>> Sean Anderson asks:
>>>
>>> "So nothing here is probed, but additionally nothing has UCLASS_TIMER.
>>> What do you expect the timer device to be?"
>>
>> There are timers on the board. How come it used to work, before the
>> commit that breaks it ?
>>
>> I understand that nobody registers a driver in the UCLASS_TIMER , but
>> why was this enforced? and if this enforcement breaks our board, we can
>> either:
>> 1/ stop the enforcement
>> 2/ comply
>>
>> Either way, I think we have to see how we can get one timer to register
>> itself as an UCLASS_TIMER
>>
>>
>>>
>>> Is the timer missing for SAMA5D3 board? I cannot find it in
>>> /drivers/timer, other than mchp-pit64b-timer.c, which doesn't
>>> seem to be used ins this particular board as far as I could tell.
>>
>> This timer is a new hardware timer which is not present in the sama5d3 SoC.
>>
>> Sama5d3 should have old PIT timers, (programmable interrupt timer)
>> and most likely that code has not been converted to DM (UCLASSes)
>>
>> One possible approach is to convert specific old PIT code to comply with
>> DM requirements (or if that code is already available, we have to
>> use/enable it ).
>>
>> I add Claudiu in the mail thread, as he is more familiar with PIT and
>> timers than I am, and maybe he has some opinion related to this issue.
>>
>> Eugen
>>
>>>
>>> Any thoughts on how to fix this? Anything I can do to help?
>>>
>>> Cheers
>>>
>>>
>>> On Tue, 23 Mar 2021 at 13:20, Manuel Luís Reis >> > wrote:
>>>
>>>
>>>  > The change may be dedicated to sama5d2 devices. Could you have a look
>>>  > please if your device (sama5d3) needs this change as well ? I
>>> mean, does
>>>  > doing something similar for sama5d3 fixes your problem ?
>>>
>>> I  am not quite sure how to check what you suggest to be honest.
>>>
>>> The commit you've sent seems to be board independent ->
>>> mach-at91/spl_atmel.c. Doesn't it apply to all of the at91 boards,
>>> sama5d3 inclusive?
>>> I don't see where else I could make a change like that.
>>>
>>> Thanks for your patience.
>>>
>>>
>>>
>>> On Tue, 23 Mar 2021 at 11:38, >> > wrote:
>>>
>>> On 3/23/21 1:28 PM, Manuel Luís Reis wrote:
>>>
>>>  > Hi,
>>>  >
>>>  > Thanks for your reply.
>>>  >
>>>  >  > Can you please check if this commit is in your tree, or,
>>> if the same has
>>>  >  > to be applied in your case (sama5d3), to make it work ?
>>>  >
>>>  > I've got that change in my tree, but I'm still getting the
>>> error message.
>>>
>>> The change may be dedicated to sama5d2 devices. Could you have a
>>> look
>>> please if your device (sama5d3) needs this change as well ? I
>>> mean, does
>>> doing something similar for sama5d3 fixes your problem ?
>>>
>>> Thanks,
>>> Eugen
>>>
>>>  >
>>>  > I 

Re: [PATCH 2/2] pinctrl: at91-pio4: add support for slew-rate

[Claudiu.Beznea]

Hi Eugen,

On 23.02.2021 11:41, Eugen Hristev - M18282 wrote:
> On 27.01.2021 15:00, Claudiu Beznea wrote:
>> SAMA7G5 supports slew rate configuration. Adapt the driver for this.
>> For switching frequencies lower than 50MHz the slew rate needs to
>> be enabled. Since most of the pins on SAMA7G5 fall into this category
>> enabled the slew rate by default.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
>>   arch/arm/mach-at91/include/mach/atmel_pio4.h |  1 +
>>   drivers/pinctrl/pinctrl-at91-pio4.c  | 26 
>> +++---
>>   2 files changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/include/mach/atmel_pio4.h 
>> b/arch/arm/mach-at91/include/mach/atmel_pio4.h
>> index 35ac7b2d40e1..c3bd9140dfef 100644
>> --- a/arch/arm/mach-at91/include/mach/atmel_pio4.h
>> +++ b/arch/arm/mach-at91/include/mach/atmel_pio4.h
>> @@ -44,6 +44,7 @@ struct atmel_pio4_port {
>>   #define ATMEL_PIO_DIR_MASK BIT(8)
>>   #define ATMEL_PIO_PUEN_MASKBIT(9)
>>   #define ATMEL_PIO_PDEN_MASKBIT(10)
>> +#define ATMEL_PIO_SRBIT(11)
>>   #define ATMEL_PIO_IFEN_MASKBIT(12)
>>   #define ATMEL_PIO_IFSCEN_MASK  BIT(13)
>>   #define ATMEL_PIO_OPD_MASK BIT(14)
>> diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c 
>> b/drivers/pinctrl/pinctrl-at91-pio4.c
>> index 3a5143adc381..5c6ece745ab0 100644
>> --- a/drivers/pinctrl/pinctrl-at91-pio4.c
>> +++ b/drivers/pinctrl/pinctrl-at91-pio4.c
>> @@ -24,6 +24,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>   
>>   struct atmel_pio4_plat {
>>  struct atmel_pio4_port *reg_base;
>> +unsigned int slew_rate_support;
>>   };
>>   
>>   static const struct pinconf_param conf_params[] = {
>> @@ -35,9 +36,11 @@ static const struct pinconf_param conf_params[] = {
>>  { "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 },
>>  { "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 },
>>  { "atmel,drive-strength", PIN_CONFIG_DRIVE_STRENGTH, 0 },
>> +{ "slew-rate", PIN_CONFIG_SLEW_RATE, 0},
> Hi Claudiu,
> 
> If slew rate is enabled by default, we should have here 0 or 1 ?

It doesn't matter the value here since the driver is used with versions of
this IP that either support or not this feature. What matters is the value
of priv->slew_rate_support that is set based on the IP version. At the
moment this feature is available only on SAMA7G5. If we would rely on the
value on conf_params we could end-up setting the SR bit for versions of
this IP that doesn't support the SR feature. Please share if you see it
implemented otherwise.

> 
>>   };
>>   
>> -static u32 atmel_pinctrl_get_pinconf(struct udevice *config)
>> +static u32 atmel_pinctrl_get_pinconf(struct udevice *config,
>> + struct atmel_pio4_plat *plat)
>>   {
>>  const struct pinconf_param *params;
>>  u32 param, arg, conf = 0;
>> @@ -52,6 +55,10 @@ static u32 atmel_pinctrl_get_pinconf(struct udevice 
>> *config)
>>  param = params->param;
>>  arg = params->default_value;
>>   
>> +/* Keep slew rate enabled by default. */
>> +if (plat->slew_rate_support)
>> +conf |= ATMEL_PIO_SR;
>> +
>>  switch (param) {
>>  case PIN_CONFIG_BIAS_DISABLE:
>>  conf &= (~ATMEL_PIO_PUEN_MASK);
>> @@ -90,6 +97,15 @@ static u32 atmel_pinctrl_get_pinconf(struct udevice 
>> *config)
>>  conf |= (val << ATMEL_PIO_DRVSTR_OFFSET)
>>  & ATMEL_PIO_DRVSTR_MASK;
>>  break;
>> +case PIN_CONFIG_SLEW_RATE:
>> +if (!plat->slew_rate_support)
>> +break;
>> +
>> +dev_read_u32(config, params->property, );
>> +/* And disable it if requested. */
>> +if (val == 0)
>> +conf &= ~ATMEL_PIO_SR;
>> +break;
>>  default:
>>  printf("%s: Unsupported configuration parameter: %u\n",
>> __func__, param);
>> @@ -115,6 +131,7 @@ static inline struct atmel_pio4_port 
>> *atmel_pio4_bank_base(struct udevice *dev,
>>   
>>   static int atmel_pinctrl_set_state(struct udevice *dev, struct udevice 
>> *config)
>>   {
>> +struct atmel_pio4_plat *plat = dev_get_plat(dev);
>>  struct atmel_pio4_port *bank_base;
>>  const void *blob = gd->fdt_blob;
>>  int node = dev_of_offset(config);
>> @@ -123,7 +140,7 @@ static int atmel_pinctrl_set_state(struct udevice *dev, 
>> struct udevice *config)
>>  u32 i, conf;
>>  int count;
>>   
>> -conf = atmel_pinctrl_get_pinconf(config);
>> +conf = atmel_pinctrl_get_pinconf(config, plat);
>>   
>>  count = fdtdec_get_int_array_count(blob, node, "pinmux",
>> cells, ARRAY_SIZE(cells));
>> @@ -163,6 +180,7 @@ const struct 

Re: [PATCH v2 2/2] timer: Return count from timer_ops.get_count

[Claudiu.Beznea]

On 07.10.2020 21:37, Sean Anderson wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> No timer drivers return an error from get_count. Instead of possibly
> returning an error, just return the count directly.
> 
> Signed-off-by: Sean Anderson 

For mchp-pit64b-timer:
Reviewed-by: Claudiu Beznea 

> ---
> 
> Changes in v2:
> - mchp-pit64b was added since v1, so convert it
> - Document when get_count may be called, and what assumptions the timer
>   subsystem makes about drivers
> 
>  arch/riscv/lib/andes_plmt.c   |  6 ++
>  arch/riscv/lib/sifive_clint.c |  6 ++
>  drivers/timer/ag101p_timer.c  |  5 ++---
>  drivers/timer/altera_timer.c  |  6 ++
>  drivers/timer/arc_timer.c |  6 ++
>  drivers/timer/ast_timer.c |  6 ++
>  drivers/timer/atcpit100_timer.c   |  5 ++---
>  drivers/timer/atmel_pit_timer.c   |  6 ++
>  drivers/timer/cadence-ttc.c   |  6 ++
>  drivers/timer/dw-apb-timer.c  |  6 ++
>  drivers/timer/mchp-pit64b-timer.c |  6 ++
>  drivers/timer/mpc83xx_timer.c |  6 ++
>  drivers/timer/mtk_timer.c |  6 ++
>  drivers/timer/nomadik-mtu-timer.c |  6 ++
>  drivers/timer/omap-timer.c|  6 ++
>  drivers/timer/ostm_timer.c|  6 ++
>  drivers/timer/riscv_timer.c   | 21 +
>  drivers/timer/rockchip_timer.c|  5 ++---
>  drivers/timer/sandbox_timer.c |  6 ++
>  drivers/timer/sti-timer.c |  6 ++
>  drivers/timer/stm32_timer.c   |  6 ++
>  drivers/timer/timer-uclass.c  |  3 ++-
>  drivers/timer/tsc_timer.c |  6 ++
>  include/timer.h   |  9 ++---
>  24 files changed, 59 insertions(+), 97 deletions(-)
> 
> diff --git a/arch/riscv/lib/andes_plmt.c b/arch/riscv/lib/andes_plmt.c
> index a28c14c1eb..cec86718c7 100644
> --- a/arch/riscv/lib/andes_plmt.c
> +++ b/arch/riscv/lib/andes_plmt.c
> @@ -17,11 +17,9 @@
>  /* mtime register */
>  #define MTIME_REG(base)((ulong)(base))
> 
> -static int andes_plmt_get_count(struct udevice *dev, u64 *count)
> +static u64 andes_plmt_get_count(struct udevice *dev)
>  {
> -   *count = readq((void __iomem *)MTIME_REG(dev->priv));
> -
> -   return 0;
> +   return readq((void __iomem *)MTIME_REG(dev->priv));
>  }
> 
>  static const struct timer_ops andes_plmt_ops = {
> diff --git a/arch/riscv/lib/sifive_clint.c b/arch/riscv/lib/sifive_clint.c
> index c9704c596f..a5572cb825 100644
> --- a/arch/riscv/lib/sifive_clint.c
> +++ b/arch/riscv/lib/sifive_clint.c
> @@ -62,11 +62,9 @@ int riscv_get_ipi(int hart, int *pending)
> return 0;
>  }
> 
> -static int sifive_clint_get_count(struct udevice *dev, u64 *count)
> +static u64 sifive_clint_get_count(struct udevice *dev)
>  {
> -   *count = readq((void __iomem *)MTIME_REG(dev->priv));
> -
> -   return 0;
> +   return readq((void __iomem *)MTIME_REG(dev->priv));
>  }
> 
>  static const struct timer_ops sifive_clint_ops = {
> diff --git a/drivers/timer/ag101p_timer.c b/drivers/timer/ag101p_timer.c
> index c011906b93..23ad5b2b67 100644
> --- a/drivers/timer/ag101p_timer.c
> +++ b/drivers/timer/ag101p_timer.c
> @@ -62,14 +62,13 @@ struct atftmr_timer_platdata {
> struct atftmr_timer_regs *regs;
>  };
> 
> -static int atftmr_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 atftmr_timer_get_count(struct udevice *dev)
>  {
> struct atftmr_timer_platdata *plat = dev->platdata;
> struct atftmr_timer_regs *const regs = plat->regs;
> u32 val;
> val = readl(>t3_counter);
> -   *count = timer_conv_64(val);
> -   return 0;
> +   return timer_conv_64(val);
>  }
> 
>  static int atftmr_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/altera_timer.c b/drivers/timer/altera_timer.c
> index 6cb2923e0b..ccc164ee17 100644
> --- a/drivers/timer/altera_timer.c
> +++ b/drivers/timer/altera_timer.c
> @@ -32,7 +32,7 @@ struct altera_timer_platdata {
> struct altera_timer_regs *regs;
>  };
> 
> -static int altera_timer_get_count(struct udevice *dev, u64 *count)
> +static u64 altera_timer_get_count(struct udevice *dev)
>  {
> struct altera_timer_platdata *plat = dev->platdata;
> struct altera_timer_regs *const regs = plat->regs;
> @@ -44,9 +44,7 @@ static int altera_timer_get_count(struct udevice *dev, u64 
> *count)
> /* Read timer value */
> val = readl(>snapl) & 0x;
> val |= (readl(>snaph) & 0x) << 16;
> -   *count = timer_conv_64(~val);
> -
> -   return 0;
> +   return timer_conv_64(~val);
>  }
> 
>  static int altera_timer_probe(struct udevice *dev)
> diff --git a/drivers/timer/arc_timer.c b/drivers/timer/arc_timer.c
> index 8c574ec5af..2dea9f40cb 100644
> --- a/drivers/timer/arc_timer.c
> +++ b/drivers/timer/arc_timer.c
> @@ -26,7 +26,7 @@ struct arc_timer_priv {
> uint timer_id;
>  };
> 
> 

Re: [PATCH] timer: mchp-pit64b: add support for pit64b

[Claudiu.Beznea]

On 07.10.2020 20:49, Sean Anderson wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> On 9/7/20 11:36 AM, Claudiu Beznea wrote:
>> Add support for Microchip PIT64B timer. The timer is 64 bit length and
>> is used as a free running counter (in continuous mode with highest values
>> for period registers). The clock feeding the timer would be no more
>> than 12.5MHz.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
>>  drivers/timer/Kconfig |   7 +++
>>  drivers/timer/Makefile|   1 +
>>  drivers/timer/mchp-pit64b-timer.c | 109 
>> ++
>>  3 files changed, 117 insertions(+)
>>  create mode 100644 drivers/timer/mchp-pit64b-timer.c
>>
>> diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
>> index 5f4bc6edb67b..13a98be4ab92 100644
>> --- a/drivers/timer/Kconfig
>> +++ b/drivers/timer/Kconfig
>> @@ -181,4 +181,11 @@ config MTK_TIMER
>> Select this to enable support for the timer found on
>> MediaTek devices.
>>
>> +config MCHP_PIT64B_TIMER
>> + bool "Microchip 64-bit periodic interval timer support"
>> + depends on TIMER
>> + help
>> +   Select this to enable support for Microchip 64-bit periodic
>> +   interval timer.
>> +
>>  endmenu
>> diff --git a/drivers/timer/Makefile b/drivers/timer/Makefile
>> index fa35bea6c5b2..4744a8d9c93c 100644
>> --- a/drivers/timer/Makefile
>> +++ b/drivers/timer/Makefile
>> @@ -21,3 +21,4 @@ obj-$(CONFIG_STI_TIMER) += sti-timer.o
>>  obj-$(CONFIG_STM32_TIMER)+= stm32_timer.o
>>  obj-$(CONFIG_X86_TSC_TIMER)  += tsc_timer.o
>>  obj-$(CONFIG_MTK_TIMER)  += mtk_timer.o
>> +obj-$(CONFIG_MCHP_PIT64B_TIMER)  += mchp-pit64b-timer.o
>> diff --git a/drivers/timer/mchp-pit64b-timer.c 
>> b/drivers/timer/mchp-pit64b-timer.c
>> new file mode 100644
>> index ..ead8c9b84ad5
>> --- /dev/null
>> +++ b/drivers/timer/mchp-pit64b-timer.c
>> @@ -0,0 +1,109 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * 64-bit Periodic Interval Timer driver
>> + *
>> + * Copyright (C) 2020 Microchip Technology Inc. and its subsidiaries
>> + *
>> + * Author: Claudiu Beznea 
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define MCHP_PIT64B_CR   0x00/* Control Register */
>> +#define  MCHP_PIT64B_CR_STARTBIT(0)
>> +#define  MCHP_PIT64B_CR_SWRSTBIT(8)
>> +#define MCHP_PIT64B_MR   0x04/* Mode Register */
>> +#define  MCHP_PIT64B_MR_CONT BIT(0)
>> +#define MCHP_PIT64B_LSB_PR   0x08/* LSB Period Register */
>> +#define MCHP_PIT64B_MSB_PR   0x0C/* MSB Period Register */
>> +#define MCHP_PIT64B_TLSBR0x20/* Timer LSB Register */
>> +#define MCHP_PIT64B_TMSBR0x24/* Timer MSB Register */
>> +
>> +struct mchp_pit64b_priv {
>> + void __iomem *base;
>> +};
>> +
>> +static int mchp_pit64b_get_count(struct udevice *dev, u64 *count)
>> +{
>> + struct mchp_pit64b_priv *priv = dev_get_priv(dev);
>> +
>> + u32 lsb = readl(priv->base + MCHP_PIT64B_TLSBR);
>> + u32 msb = readl(priv->base + MCHP_PIT64B_TMSBR);
> 
> Is there a chance of rollover here? E.g. lets say the 64-bit counter is
> 0x___ when we read lsb, but it increments to
> 0x_0001__ when we read msb. Then the next time we read the
> timer, it will look like we jumped backward in time.

This should not happen as the hardware takes care of it as follows:
when using a 64 bit period TLSB must be read first, followed by the
read of TMSB. This sequence generates an atomic read of the 64 bit
timer value whatever the lapse of time between the accesses.

Thank you,
Claudiu Beznea

> 
> One way to get around this is by doing something like
> 
> do {
> msb = readl(priv->base + MCHP_PIT64B_TMSBR);
> lsb = readl(priv->base + MCHP_PIT64B_TLSBR);
> } while (msb != readl(priv->base + MCHP_PIT64B_TMSBR));
> 
> That way, if we ever roll-over between the two reads, we just redo the
> lsb read. This is the method used by drivers/timer/riscv_timer.c on
> 32-bit systems.
> 
> --Sean
> 
>> +
>> + *count = ((u64)msb << 32) | lsb;
>> +
>> + return 0;
>> +}
>> +
>> +static int mchp_pit64b_probe(struct udevice *dev)
>> +{
>> + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>> + struct mchp_pit64b_priv *priv = dev_get_priv(dev);
>> + struct clk clk;
>> + ulong rate;
>> + int ret;
>> +
>> + priv->base = dev_read_addr_ptr(dev);
>> + if (IS_ERR(priv->base))
>> + return PTR_ERR(priv->base);
>> +
>> + ret = clk_get_by_index(dev, 0, );
>> + if (ret)
>> + return ret;
>> +
>> + ret = clk_enable();
>> + if (ret)
>> + return ret;
>> +
>> + rate = clk_get_rate();
>> + if (!rate) {
>> + clk_disable();
>> +  

Re: [PATCH 2/7] clk: at91: sam9x60: add support compatible with CCF

[Claudiu.Beznea]

Hi Eugen,


On 06.10.2020 09:32, Eugen Hristev - M18282 wrote:
> On 05.10.2020 17:58, Claudiu Beznea wrote:
>> Add SAM9X60 clock support compatible with CCF.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
> 
> Hi Claudiu,
> 
>>   drivers/clk/at91/Makefile  |   1 +
>>   drivers/clk/at91/sam9x60.c | 594 
>> +
>>   2 files changed, 595 insertions(+)
>>   create mode 100644 drivers/clk/at91/sam9x60.c
>>
>> diff --git a/drivers/clk/at91/Makefile b/drivers/clk/at91/Makefile
>> index 2453c38af1aa..580b406d7bd6 100644
>> --- a/drivers/clk/at91/Makefile
>> +++ b/drivers/clk/at91/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_AT91_GENERIC_CLK) += clk-generic.o
>>   obj-$(CONFIG_AT91_UTMI)+= clk-utmi.o
>>   obj-$(CONFIG_AT91_SAM9X60_PLL) += clk-sam9x60-pll.o
>>   obj-$(CONFIG_SAMA7G5)  += sama7g5.o
>> +obj-$(CONFIG_SAM9X60)   += sam9x60.o
>>   else
>>   obj-y += compat.o
>>   endif
>> diff --git a/drivers/clk/at91/sam9x60.c b/drivers/clk/at91/sam9x60.c
>> new file mode 100644
>> index ..10ef85fca2cf
>> --- /dev/null
>> +++ b/drivers/clk/at91/sam9x60.c
>> @@ -0,0 +1,594 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2020 Microchip Technology Inc. and its subsidiaries
>> + *
>> + * Author: Claudiu Beznea 
>> + *
>> + * Based on sam9x60.c on Linux.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "pmc.h"
>> +
>> +/**
>> + * Clock identifiers to be used in conjunction with macros like
>> + * AT91_TO_CLK_ID()
>> + *
>> + * @ID_MD_SLCK: TD slow clock identifier
>> + * @ID_TD_SLCK: MD slow clock identifier
>> + * @ID_MAIN_XTAL:   Main Xtal clock identifier
>> + * @ID_MAIN_RC: Main RC clock identifier
>> + * @ID_MAIN_RC_OSC: Main RC Oscillator clock identifier
>> + * @ID_MAIN_OSC:Main Oscillator clock identifier
>> + * @ID_MAINCK:  MAINCK clock identifier
>> + * @ID_PLL_U_FRAC:  UPLL fractional clock identifier
>> + * @ID_PLL_U_DIV:   UPLL divider clock identifier
>> + * @ID_PLL_A_FRAC:  APLL fractional clock identifier
>> + * @ID_PLL_A_DIV:   APLL divider clock identifier
>> +
>> + * @ID_MCK: MCK clock identifier
>> +
>> + * @ID_UTMI:UTMI clock identifier
>> +
>> + * @ID_PROG0:   Programmable 0 clock identifier
>> + * @ID_PROG1:   Programmable 1 clock identifier
>> +
>> + * @ID_PCK0:PCK0 system clock identifier
>> + * @ID_PCK1:PCK1 system clock identifier
>> + * @ID_DDR: DDR system clock identifier
>> + * @ID_QSPI:QSPI system clock identifier
>> + *
>> + * Note: if changing the values of this enums please sync them with
>> + *   device tree
>> + */
>> +enum pmc_clk_ids {
>> +ID_MD_SLCK  = 0,
>> +ID_TD_SLCK  = 1,
>> +ID_MAIN_XTAL= 2,
>> +ID_MAIN_RC  = 3,
>> +ID_MAIN_RC_OSC  = 4,
>> +ID_MAIN_OSC = 5,
>> +ID_MAINCK   = 6,
>> +
>> +ID_PLL_U_FRAC   = 7,
>> +ID_PLL_U_DIV= 8,
>> +ID_PLL_A_FRAC   = 9,
>> +ID_PLL_A_DIV= 10,
>> +
>> +ID_MCK  = 11,
>> +
>> +ID_UTMI = 12,
>> +
>> +ID_PROG0= 13,
>> +ID_PROG1= 14,
>> +
>> +ID_PCK0 = 15,
>> +ID_PCK1 = 16,
>> +
>> +ID_DDR  = 17,
>> +ID_QSPI = 18,
>> +
>> +ID_MAX,
>> +};
>> +
>> +/**
>> + * PLL type identifiers
>> + * @PLL_TYPE_FRAC:  fractional PLL identifier
>> + * @PLL_TYPE_DIV:   divider PLL identifier
>> + */
>> +enum pll_type {
>> +PLL_TYPE_FRAC,
>> +PLL_TYPE_DIV,
>> +};
>> +
>> +/* Clock names used as parents for multiple clocks. */
>> +static const char *clk_names[] = {
>> +[ID_MAIN_RC_OSC]= "main_rc_osc",
>> +[ID_MAIN_OSC]   = "main_osc",
>> +[ID_MAINCK] = "mainck",
>> +[ID_PLL_U_DIV]  = "upll_divpmcck",
>> +[ID_PLL_A_DIV]  = "plla_divpmcck",
>> +[ID_MCK]= "mck",
>> +};
>> +
>> +/* Fractional PLL output range. */
>> +static const struct clk_range plla_outputs[] = {
>> +{ .min = 2343750, .max = 12 },
>> +};
>> +
>> +static const struct clk_range upll_outputs[] = {
>> +{ .min = 3, .max = 5 },
>> +};
>> +
>> +/* PLL characteristics. */
>> +static const struct clk_pll_characteristics apll_characteristics = {
>> +.input = { .min = 1200, .max = 4800 },
>> +.num_output = ARRAY_SIZE(plla_outputs),
>> +.output = plla_outputs,
>> +};
>> +
>> +static const struct clk_pll_characteristics upll_characteristics = {
>> +.input = { .min = 1200, .max = 

Re: [PATCH 4/7] ARM: dts: sam9x60: use u-boot,dm-pre-reloc

[Claudiu.Beznea]

Hi Eugen,

On 06.10.2020 09:39, Eugen Hristev - M18282 wrote:
> On 05.10.2020 17:58, Claudiu Beznea wrote:
>> Use u-boot,dm-pre-reloc for slow xtal and main xtal.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
> 
> Hi Claudiu,
> 
> For this patch and the following DT patches:
> 
> the u-boot,dm-pre-reloc property must be added to the file 
> sam9x60ek-u-boot.dtsi (or sam9x60-u-boot.dtsi if needed to be created )

For clocks section in device tree (which contains these fixed clocks) there
is no entry in sam9x60ek-u-boot.dtsi. Would you like me to add
u-boot,dm-pre-reloc only per section or for each clock in the clocks section?

Thank you,
Claudiu Beznea

> 
> Eugen
> 
>>   arch/arm/dts/sam9x60.dtsi | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/dts/sam9x60.dtsi b/arch/arm/dts/sam9x60.dtsi
>> index 51de586e1900..a4e2576d8e0f 100644
>> --- a/arch/arm/dts/sam9x60.dtsi
>> +++ b/arch/arm/dts/sam9x60.dtsi
>> @@ -30,11 +30,13 @@
>>  slow_xtal: slow_xtal {
>>  compatible = "fixed-clock";
>>  #clock-cells = <0>;
>> +u-boot,dm-pre-reloc;
>>  };
>>   
>>  main_xtal: main_xtal {
>>  compatible = "fixed-clock";
>>  #clock-cells = <0>;
>> +u-boot,dm-pre-reloc;
>>  };
>>  };
>>   
>>
> 

Re: [PATCH] timer: mchp-pit64b: add support for pit64b

[Claudiu.Beznea]

Hi Eugen,

On 22.09.2020 11:32, Eugen Hristev - M18282 wrote:
> On 07.09.2020 18:36, Claudiu Beznea wrote:
>> Add support for Microchip PIT64B timer. The timer is 64 bit length and
>> is used as a free running counter (in continuous mode with highest values
>> for period registers). The clock feeding the timer would be no more
>> than 12.5MHz.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
> 
> Hi Claudiu,
> 
> To make CI/CD scripts happy, this new driver needs a MAINTAINERS entry

Would you like me to create a new entry or add the file under the
ARM MICROCHIP/ATMEL AT91 hood?

Thank you,
Claudiu Beznea

> 
> Thanks !
> Eugen
> 

Re: [PATCH v2 03/21] clk: bind clk to new parent device

[Claudiu.Beznea]

Hi Simon,

On 05.08.2020 18:11, Claudiu Beznea wrote:
> Clock re-parenting is not binding the clock's device to its new
> parent device, it only calls the clock's ops->set_parent() API. The
> changes in this commit re-parent the clock device to its new parent
> so that subsequent operations like clk_get_parent() to point to the
> proper parent.
> 
> Signed-off-by: Claudiu Beznea 
> Reviewed-by: Simon Glass 

I added your Reviewed-by here but at the same time I also added the sandbox
test in this patch. Let me know if you want the sandbox test in a separate
patch. Same thing for patch 4/21 in this series.

Thank you,
Claudiu Beznea

> ---
>  drivers/clk/clk-uclass.c | 11 ++-
>  test/dm/clk_ccf.c| 27 +++
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index 934cd5787a5c..5e0c8419d65b 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -512,6 +513,7 @@ ulong clk_set_rate(struct clk *clk, ulong rate)
>  int clk_set_parent(struct clk *clk, struct clk *parent)
>  {
>   const struct clk_ops *ops;
> + int ret;
>  
>   debug("%s(clk=%p, parent=%p)\n", __func__, clk, parent);
>   if (!clk_valid(clk))
> @@ -521,7 +523,14 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
>   if (!ops->set_parent)
>   return -ENOSYS;
>  
> - return ops->set_parent(clk, parent);
> + ret = ops->set_parent(clk, parent);
> + if (ret)
> + return ret;
> +
> + if (CONFIG_IS_ENABLED(CLK_CCF))
> + ret = device_reparent(clk->dev, parent->dev);
> +
> + return ret;
>  }
>  
>  int clk_enable(struct clk *clk)
> diff --git a/test/dm/clk_ccf.c b/test/dm/clk_ccf.c
> index da2292a51a95..b039cc5f8d8f 100644
> --- a/test/dm/clk_ccf.c
> +++ b/test/dm/clk_ccf.c
> @@ -22,6 +22,10 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
>   struct udevice *dev;
>   long long rate;
>   int ret;
> +#if CONFIG_IS_ENABLED(CLK_CCF)
> + const char *clkname;
> + int clkid;
> +#endif
>  
>   /* Get the device using the clk device */
>   ut_assertok(uclass_get_device_by_name(UCLASS_CLK, "clk-ccf", ));
> @@ -91,6 +95,29 @@ static int dm_test_clk_ccf(struct unit_test_state *uts)
>  
>   ret = sandbox_clk_enable_count(pclk);
>   ut_asserteq(ret, 0);
> +
> + /* Test clock re-parenting. */
> + ret = clk_get_by_id(SANDBOX_CLK_USDHC1_SEL, );
> + ut_assertok(ret);
> + ut_asserteq_str("usdhc1_sel", clk->dev->name);
> +
> + pclk = clk_get_parent(clk);
> + ut_assertok_ptr(pclk);
> + if (!strcmp(pclk->dev->name, "pll3_60m")) {
> + clkname = "pll3_80m";
> + clkid = SANDBOX_CLK_PLL3_80M;
> + } else {
> + clkname = "pll3_60m";
> + clkid = SANDBOX_CLK_PLL3_60M;
> + }
> +
> + ret = clk_get_by_id(clkid, );
> + ut_assertok(ret);
> + ret = clk_set_parent(clk, pclk);
> + ut_assertok(ret);
> + pclk = clk_get_parent(clk);
> + ut_assertok_ptr(pclk);
> + ut_asserteq_str(clkname, pclk->dev->name);
>  #endif
>  
>   return 1;
> 

Re: [PATCH 01/22] clk: check hw and hw->dev before dereference it

[Claudiu.Beznea]

On 04.08.2020 18:29, Simon Glass wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> Hi Claudiu,
> 
> On Tue, 4 Aug 2020 at 09:25,  wrote:
>>
>> Hi Simon,
>>
>> On 04.08.2020 18:08, Simon Glass wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
>>> content is safe
>>>
>>> Hi Claudiu,
>>>
>>> On Tue, 4 Aug 2020 at 01:19,  wrote:



 On 04.08.2020 05:00, Simon Glass wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> the content is safe
>
> Hi Claudiu,
>
> On Wed, 29 Jul 2020 at 08:51, Claudiu Beznea
>  wrote:
>>
>> Check hw and hw->dev before dereference it.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
>>  drivers/clk/clk.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>
> Why is this needed? It adds to code size and these situations should
> not occur. Perhaps use assert()?

 In my debugging, investigating the issues that patches 03/22, 04/22, 06/22
 try to address, I reached also this function and checked these pointers. In
 the end the issue was not related to them but I though it might be useful
 to keep these in a patch. I will remove it in the next version.
>>>
>>> IMO we should use assert() to check invariants and catch basic
>>> programming errors. But production testing should make sure that the
>>> software basically works.
>>>
>>> Of course it is nice to have these checks, but they add to code size
>>> which is always a concern. So I think we should rely on assert() to
>>> catch the errors during development, so we are not wasting code
>>> checking for things that we know cannot happen.
>>
>> OK, I'll switch to assert().
> 
> One more point I should have made is that my comments apply mostly to
> common code that everyone has to use - e.g. the core clock code. So if
> you want to put dev_err() and other things in your driver and you know
> about the code-size implications that is less of a concern. But with
> common code, we should be careful.

Sure!

Thank you,
Claudiu Beznea

> 
> Regards,
> Simon
> 

Re: [PATCH 01/22] clk: check hw and hw->dev before dereference it

[Claudiu.Beznea]

Hi Simon,

On 04.08.2020 18:08, Simon Glass wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> Hi Claudiu,
> 
> On Tue, 4 Aug 2020 at 01:19,  wrote:
>>
>>
>>
>> On 04.08.2020 05:00, Simon Glass wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
>>> content is safe
>>>
>>> Hi Claudiu,
>>>
>>> On Wed, 29 Jul 2020 at 08:51, Claudiu Beznea
>>>  wrote:

 Check hw and hw->dev before dereference it.

 Signed-off-by: Claudiu Beznea 
 ---
  drivers/clk/clk.c | 3 +++
  1 file changed, 3 insertions(+)

>>>
>>> Why is this needed? It adds to code size and these situations should
>>> not occur. Perhaps use assert()?
>>
>> In my debugging, investigating the issues that patches 03/22, 04/22, 06/22
>> try to address, I reached also this function and checked these pointers. In
>> the end the issue was not related to them but I though it might be useful
>> to keep these in a patch. I will remove it in the next version.
> 
> IMO we should use assert() to check invariants and catch basic
> programming errors. But production testing should make sure that the
> software basically works.
> 
> Of course it is nice to have these checks, but they add to code size
> which is always a concern. So I think we should rely on assert() to
> catch the errors during development, so we are not wasting code
> checking for things that we know cannot happen.

OK, I'll switch to assert().

Thank you,
Claudiu Beznea

> 
> Regards,
> Simon
> 

Re: [PATCH 06/22] clk: get clock pointer before proceeding

[Claudiu.Beznea]

On 04.08.2020 05:00, Simon Glass wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> Hi Claudiu,
> 
> On Wed, 29 Jul 2020 at 08:52, Claudiu Beznea
>  wrote:
>>
>> clk_get_by_indexed_prop() retrieves a clock with dev member being set
>> with the pointer to the udevice for the clock controller driver. But
>> in case of CCF each struct clk object has set in dev member the reference
>> to its parent (the root of the clock tree is a fixed clock, every
>> node in clock tree is a clock registered with clk_register()). In this
>> case the subsequent operations like dev_get_clk_ptr() on clocks
>> retrieved by clk_get_by_indexed_prop() will fail. For this, get the
>> pointer to the proper clock registered (with clk_register()) using
>> clk_get_by_id() before proceeding.
>>
>> Fixes: 1d7993d1d0ef ("clk: Port Linux common clock framework [CCF] for imx6q 
>> to U-boot (tag: v5.1.12)")
>> Signed-off-by: Claudiu Beznea 
>> ---
>>  drivers/clk/clk-uclass.c | 41 +
>>  1 file changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>> index 958a9490bee2..8f926aad12cf 100644
>> --- a/drivers/clk/clk-uclass.c
>> +++ b/drivers/clk/clk-uclass.c
>> @@ -186,7 +186,7 @@ bulk_get_err:
>>
>>  static int clk_set_default_parents(struct udevice *dev, int stage)
>>  {
>> -   struct clk clk, parent_clk;
>> +   struct clk clk, parent_clk, *c, *p;
>> int index;
>> int num_parents;
>> int ret;
>> @@ -212,6 +212,17 @@ static int clk_set_default_parents(struct udevice *dev, 
>> int stage)
>> return ret;
>> }
>>
>> +   if (CONFIG_IS_ENABLED(CLK_CCF)) {
>> +   ret = clk_get_by_id(parent_clk.id, );
>> +   if (ret) {
>> +   debug("%s(): could not get parent clock 
>> pointer, id %lu, for %s\n",
>> + __func__, parent_clk.id, 
>> dev_read_name(dev));
>> +   return ret;
>> +   }
>> +   } else {
>> +   p = _clk;
>> +   }
>> +
>> ret = clk_get_by_indexed_prop(dev, "assigned-clocks",
>>   index, );
>> if (ret) {
>> @@ -231,7 +242,18 @@ static int clk_set_default_parents(struct udevice *dev, 
>> int stage)
>> /* do not setup twice the parent clocks */
>> continue;
>>
>> -   ret = clk_set_parent(, _clk);
>> +   if (CONFIG_IS_ENABLED(CLK_CCF)) {
>> +   ret = clk_get_by_id(clk.id, );
>> +   if (ret) {
>> +   debug("%s(): could not get clock pointer, id 
>> %lu, for %s\n",
>> + __func__, clk.id, dev_read_name(dev));
>> +   return ret;
>> +   }
>> +   } else {
>> +   c = 
>> +   }
> 
> Could this code go in a function? It seems to be repeated three times.

Sure, it will!

Thank you reviewing this,
Claudiu Beznea

> 
>> +
>> +   ret = clk_set_parent(c, p);
>> /*
>>  * Not all drivers may support clock-reparenting (as of now).
>>  * Ignore errors due to this.
>> @@ -251,7 +273,7 @@ static int clk_set_default_parents(struct udevice *dev, 
>> int stage)
>>
>>  static int clk_set_default_rates(struct udevice *dev, int stage)
>>  {
>> -   struct clk clk;
>> +   struct clk clk, *c;
>> int index;
>> int num_rates;
>> int size;
>> @@ -295,7 +317,18 @@ static int clk_set_default_rates(struct udevice *dev, 
>> int stage)
>> /* do not setup twice the parent clocks */
>> continue;
>>
>> -   ret = clk_set_rate(, rates[index]);
>> +   if (CONFIG_IS_ENABLED(CLK_CCF)) {
>> +   ret = clk_get_by_id(clk.id, );
>> +   if (ret) {
>> +   debug("%s(): could not get clock pointer, id 
>> %lu, for %s\n",
>> + __func__, clk.id, dev_read_name(dev));
>> +   return ret;
>> +   }
>> +   } else {
>> +   c = 
>> +   }
>> +
>> +   ret = clk_set_rate(c, rates[index]);
>>
>> if (ret < 0) {
>> debug("%s: failed to set rate on clock index %d 
>> (%ld) for %s\n",
>> --
>> 2.7.4
>>
> 
> Regards,
> Simon
> 

Re: [PATCH 03/22] dm: core: add support for device re-parenting

[Claudiu.Beznea]

On 04.08.2020 05:00, Simon Glass wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> Hi Claudiu,
> 
> On Wed, 29 Jul 2020 at 08:51, Claudiu Beznea
>  wrote:
>>
>> In common clock framework the relation b/w parent and child clocks is
>> determined based on the udevice parent/child information. A clock
>> parent could be changed based on devices needs. In case this is happen
>> the functionalities for clock who's parent is changed are broken. Add
>> a function that reparent a device. This will be used in clk-uclass.c
>> to reparent a clock device.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
>>  drivers/core/device.c| 26 ++
>>  include/dm/device-internal.h |  9 +
>>  2 files changed, 35 insertions(+)
> 
> Please add a sandbox test for this function.

OK.

> 
>>
>> diff --git a/drivers/core/device.c b/drivers/core/device.c
>> index a7408d9c76c6..f149d55ac1e1 100644
>> --- a/drivers/core/device.c
>> +++ b/drivers/core/device.c
>> @@ -267,6 +267,32 @@ int device_bind_by_name(struct udevice *parent, bool 
>> pre_reloc_only,
>> devp);
>>  }
>>
>> +int device_reparent(struct udevice *dev, struct udevice *new_parent)
>> +{
>> +   struct udevice *cparent;
>> +   struct udevice *pos, *n;
>> +
>> +   if (!dev || !new_parent)
>> +   return -EINVAL;
>> +
> 
> This is an error by the caller and would not be present in production
> code. Perhaps use assert()?

OK, I'll use assert().

> 
>> +   if (!dev->parent)
>> +   return -ENODEV;
> 
> This can't happen. Every device except for the root one has a parent.

Sure, I'll remove it.

> 
>> +
>> +   list_for_each_entry_safe(pos, n, >parent->child_head,
>> +sibling_node) {
>> +   if (pos->driver != dev->driver)
>> +   continue;
>> +
>> +   list_del(>sibling_node);
>> +   list_add_tail(>sibling_node, _parent->child_head);
>> +   dev->parent = new_parent;
>> +
>> +   return 0;
>> +   }
>> +
>> +   return -ENODEV;
> 
> What does this error mean?

That the device who needs re-parenting has no parent. But you already
pointed that this should not happen. This means that the above loop will
always have a match and here we should return success code.

> 
>> +}
>> +
>>  static void *alloc_priv(int size, uint flags)
>>  {
>> void *priv;
>> diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
>> index 294d6c18105a..c5d7ec0650f9 100644
>> --- a/include/dm/device-internal.h
>> +++ b/include/dm/device-internal.h
>> @@ -84,6 +84,15 @@ int device_bind_by_name(struct udevice *parent, bool 
>> pre_reloc_only,
>> const struct driver_info *info, struct udevice 
>> **devp);
>>
>>  /**
>> + * device_reparent: reparent the device to a new parent
>> + *
>> + * @dev: pointer to device to be reparented
>> + * @new_parent: pointer to new parent device
>> + * @return 0 if OK, -ve on error
>> + */
>> +int device_reparent(struct udevice *dev, struct udevice *new_parent);
>> +
>> +/**
>>   * device_ofdata_to_platdata() - Read platform data for a device
>>   *
>>   * Read platform data for a device (typically from the device tree) so that
>> --
>> 2.7.4
>>
> 
> Regards,
> Simon
> 

Re: [PATCH 04/22] clk: bind clk to new parent device

[Claudiu.Beznea]

On 04.08.2020 05:00, Simon Glass wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> On Wed, 29 Jul 2020 at 08:51, Claudiu Beznea
>  wrote:
>>
>> Clock re-parenting is not binding the clock's device to its new
>> parent device, it only calls the clock's ops->set_parent() API. The
>> changes in this commit re-parent the clock device to its new parent
>> so that subsequent operations like clk_get_parent() to point to the
>> proper parent.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
>>  drivers/clk/clk-uclass.c | 11 ++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Simon Glass 
> 
> But please add a sandbox test.

Sure!

> 
> 
>>
>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>> index aa1f11a27c41..b390a6b01c06 100644
>> --- a/drivers/clk/clk-uclass.c
>> +++ b/drivers/clk/clk-uclass.c
>> @@ -14,6 +14,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -511,6 +512,7 @@ ulong clk_set_rate(struct clk *clk, ulong rate)
>>  int clk_set_parent(struct clk *clk, struct clk *parent)
>>  {
>> const struct clk_ops *ops;
>> +   int ret;
>>
>> debug("%s(clk=%p, parent=%p)\n", __func__, clk, parent);
>> if (!clk_valid(clk))
>> @@ -520,7 +522,14 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
>> if (!ops->set_parent)
>> return -ENOSYS;
>>
>> -   return ops->set_parent(clk, parent);
>> +   ret = ops->set_parent(clk, parent);
>> +   if (ret)
>> +   return ret;
>> +
>> +   if (CONFIG_IS_ENABLED(CLK_CCF))
>> +   ret = device_reparent(clk->dev, parent->dev);
>> +
>> +   return ret;
>>  }
>>
>>  int clk_enable(struct clk *clk)
>> --
>> 2.7.4
>>

Re: [PATCH 02/22] clk: check pointer returned by dev_get_parent()

[Claudiu.Beznea]

On 04.08.2020 05:00, Simon Glass wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> Hi Claudiu,
> 
> On Wed, 29 Jul 2020 at 08:51, Claudiu Beznea
>  wrote:
>>
>> Check pointer returned by dev_get_parent().
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
>>  drivers/clk/clk-uclass.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>> index 70df9d410f4c..aa1f11a27c41 100644
>> --- a/drivers/clk/clk-uclass.c
>> +++ b/drivers/clk/clk-uclass.c
>> @@ -459,6 +459,9 @@ struct clk *clk_get_parent(struct clk *clk)
>> return NULL;
>>
>> pdev = dev_get_parent(clk->dev);
>> +   if (!pdev)
>> +   return ERR_PTR(-ENOMEM);
> 
> A clock device must always have a parent (e.g. the root device). So
> this check is not useful and adds to code size.

Sure, I'll remove it.

> >> +
>> pclk = dev_get_clk_ptr(pdev);
>> if (!pclk)
>> return ERR_PTR(-ENODEV);
>> --
>> 2.7.4
>>
> 
> Regards,
> Simon
> 

Re: [PATCH 01/22] clk: check hw and hw->dev before dereference it

[Claudiu.Beznea]

On 04.08.2020 05:00, Simon Glass wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> Hi Claudiu,
> 
> On Wed, 29 Jul 2020 at 08:51, Claudiu Beznea
>  wrote:
>>
>> Check hw and hw->dev before dereference it.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
>>  drivers/clk/clk.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
> 
> Why is this needed? It adds to code size and these situations should
> not occur. Perhaps use assert()?

In my debugging, investigating the issues that patches 03/22, 04/22, 06/22
try to address, I reached also this function and checked these pointers. In
the end the issue was not related to them but I though it might be useful
to keep these in a patch. I will remove it in the next version.

> 
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 0f55ba751c0f..9fa18e342eaf 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -57,6 +57,9 @@ ulong clk_generic_get_rate(struct clk *clk)
>>
>>  const char *clk_hw_get_name(const struct clk *hw)
>>  {
>> +   if (!hw || !hw->dev)
>> +   return NULL;
>> +
>> return hw->dev->name;
>>  }
>>
>> --
>> 2.7.4
>>
> 
> Regards,
> SImon
> 

[U-Boot] [PATCH 2/4] pinctrl: at91: add drive strength support for SAM9X60

[Claudiu.Beznea]

From: Claudiu Beznea 

Add drive strength support for SAM9X60 pin controller.

Signed-off-by: Claudiu Beznea 
---
 drivers/pinctrl/pinctrl-at91.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 8997732376e4..134ee851d978 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -248,6 +248,28 @@ static void at91_mux_sam9x5_set_drivestrength(struct 
at91_port *pio,
set_drive_strength(reg, pin, setting);
 }
 
+static void at91_mux_sam9x60_set_drivestrength(struct at91_port *pio, u32 pin,
+  u32 setting)
+{
+   void *reg = >driver12;
+   u32 tmp;
+
+   if (setting <= DRIVE_STRENGTH_BIT_DEF ||
+   setting == DRIVE_STRENGTH_BIT_MED ||
+   setting > DRIVE_STRENGTH_BIT_HI)
+   return;
+
+   tmp = readl(reg);
+
+   /* Strength is 0: low, 1: hi */
+   if (setting == DRIVE_STRENGTH_BIT_LOW)
+   tmp &= ~BIT(pin);
+   else
+   tmp |= BIT(pin);
+
+   writel(tmp, reg);
+}
+
 static struct at91_pinctrl_mux_ops at91rm9200_ops = {
.mux_A_periph   = at91_mux_set_A_periph,
.mux_B_periph   = at91_mux_set_B_periph,
@@ -278,6 +300,18 @@ static struct at91_pinctrl_mux_ops sama5d3_ops = {
.set_drivestrength = at91_mux_sama5d3_set_drivestrength,
 };
 
+static struct at91_pinctrl_mux_ops sam9x60_ops = {
+   .mux_A_periph   = at91_mux_pio3_set_A_periph,
+   .mux_B_periph   = at91_mux_pio3_set_B_periph,
+   .mux_C_periph   = at91_mux_pio3_set_C_periph,
+   .mux_D_periph   = at91_mux_pio3_set_D_periph,
+   .set_deglitch   = at91_mux_pio3_set_deglitch,
+   .set_debounce   = at91_mux_pio3_set_debounce,
+   .set_pulldown   = at91_mux_pio3_set_pulldown,
+   .disable_schmitt_trig = at91_mux_pio3_disable_schmitt_trig,
+   .set_drivestrength = at91_mux_sam9x60_set_drivestrength,
+};
+
 static void at91_mux_gpio_disable(struct at91_port *pio, u32 mask)
 {
writel(mask, >pdr);
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 3/4] pinctrl: at91: add compatibles for SAM9X60 pin controller

[Claudiu.Beznea]

From: Claudiu Beznea 

Add compatibles for SAM9X60 pin controller.

Signed-off-by: Claudiu Beznea 
---
 drivers/pinctrl/pinctrl-at91.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 134ee851d978..62ee7668ab17 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -479,6 +479,7 @@ static const struct udevice_id at91_pinctrl_match[] = {
{ .compatible = "atmel,sama5d3-pinctrl", .data = (ulong)_ops },
{ .compatible = "atmel,at91sam9x5-pinctrl", .data = 
(ulong)_ops },
{ .compatible = "atmel,at91rm9200-pinctrl", .data = 
(ulong)_ops },
+   { .compatible = "microchip,sam9x60-pinctrl", .data = 
(ulong)_ops },
{}
 };
 
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 4/4] pinctrl: at91: add slewrate support for SAM9X60

[Claudiu.Beznea]

From: Claudiu Beznea 

Add slew rate support for SAM9X60 pin controller.

Signed-off-by: Claudiu Beznea 
---
 drivers/pinctrl/pinctrl-at91.c | 34 ++
 include/dt-bindings/pinctrl/at91.h |  4 
 2 files changed, 38 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 62ee7668ab17..27f274ff0224 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -37,6 +37,9 @@ struct at91_pinctrl_priv {
 #define OUTPUT BIT(7)
 #define OUTPUT_VAL_SHIFT   8
 #define OUTPUT_VAL (0x1 << OUTPUT_VAL_SHIFT)
+#define SLEWRATE_SHIFT 9
+#define SLEWRATE_MASK  0x1
+#define SLEWRATE   (SLEWRATE_MASK << SLEWRATE_SHIFT)
 #define DEBOUNCE   BIT(16)
 #define DEBOUNCE_VAL_SHIFT 17
 #define DEBOUNCE_VAL   (0x3fff << DEBOUNCE_VAL_SHIFT)
@@ -60,6 +63,13 @@ enum drive_strength_bit {
 #define DRIVE_STRENGTH_BIT_MSK(name)   (DRIVE_STRENGTH_BIT_##name << \
 DRIVE_STRENGTH_SHIFT)
 
+enum slewrate_bit {
+   SLEWRATE_BIT_DIS,
+   SLEWRATE_BIT_ENA,
+};
+
+#define SLEWRATE_BIT_MSK(name) (SLEWRATE_BIT_##name << SLEWRATE_SHIFT)
+
 enum at91_mux {
AT91_MUX_GPIO = 0,
AT91_MUX_PERIPH_A = 1,
@@ -95,6 +105,7 @@ struct at91_pinctrl_mux_ops {
void (*disable_schmitt_trig)(struct at91_port *pio, u32 mask);
void (*set_drivestrength)(struct at91_port *pio, u32 pin,
  u32 strength);
+   void (*set_slewrate)(struct at91_port *pio, u32 pin, u32 slewrate);
 };
 
 static u32 two_bit_pin_value_shift_amount(u32 pin)
@@ -270,6 +281,25 @@ static void at91_mux_sam9x60_set_drivestrength(struct 
at91_port *pio, u32 pin,
writel(tmp, reg);
 }
 
+static void at91_mux_sam9x60_set_slewrate(struct at91_port *pio, u32 pin,
+ u32 setting)
+{
+   void *reg = >reserved12[3];
+   u32 tmp;
+
+   if (setting < SLEWRATE_BIT_DIS || setting > SLEWRATE_BIT_ENA)
+   return;
+
+   tmp = readl(reg);
+
+   if (setting == SLEWRATE_BIT_DIS)
+   tmp &= ~BIT(pin);
+   else
+   tmp |= BIT(pin);
+
+   writel(tmp, reg);
+}
+
 static struct at91_pinctrl_mux_ops at91rm9200_ops = {
.mux_A_periph   = at91_mux_set_A_periph,
.mux_B_periph   = at91_mux_set_B_periph,
@@ -310,6 +340,7 @@ static struct at91_pinctrl_mux_ops sam9x60_ops = {
.set_pulldown   = at91_mux_pio3_set_pulldown,
.disable_schmitt_trig = at91_mux_pio3_disable_schmitt_trig,
.set_drivestrength = at91_mux_sam9x60_set_drivestrength,
+   .set_slewrate   = at91_mux_sam9x60_set_slewrate,
 };
 
 static void at91_mux_gpio_disable(struct at91_port *pio, u32 mask)
@@ -378,6 +409,9 @@ static int at91_pinconf_set(struct at91_pinctrl_mux_ops 
*ops,
if (ops->set_drivestrength)
ops->set_drivestrength(pio, pin,
(config & DRIVE_STRENGTH) >> DRIVE_STRENGTH_SHIFT);
+   if (ops->set_slewrate)
+   ops->set_slewrate(pio, pin,
+   (config & SLEWRATE) >> SLEWRATE_SHIFT);
 
return 0;
 }
diff --git a/include/dt-bindings/pinctrl/at91.h 
b/include/dt-bindings/pinctrl/at91.h
index 2732d6c0fb39..616f5ce40079 100644
--- a/include/dt-bindings/pinctrl/at91.h
+++ b/include/dt-bindings/pinctrl/at91.h
@@ -17,6 +17,7 @@
 #define AT91_PINCTRL_DIS_SCHMIT(1 << 4)
 #define AT91_PINCTRL_OUTPUT(1 << 7)
 #define AT91_PINCTRL_OUTPUT_VAL(x) ((x & 0x1) << 8)
+#define AT91_PINCTRL_SLEWRATE  (1 << 9)
 #define AT91_PINCTRL_DEBOUNCE  (1 << 16)
 #define AT91_PINCTRL_DEBOUNCE_VAL(x)   (x << 17)
 
@@ -27,6 +28,9 @@
 #define AT91_PINCTRL_DRIVE_STRENGTH_MED(0x2 << 5)
 #define AT91_PINCTRL_DRIVE_STRENGTH_HI (0x3 << 5)
 
+#define AT91_PINCTRL_SLEWRATE_DIS  (0x0 << 9)
+#define AT91_PINCTRL_SLEWRATE_ENA  (0x1 << 9)
+
 #define AT91_PIOA  0
 #define AT91_PIOB  1
 #define AT91_PIOC  2
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 0/4] add drive strenght and slew rate support for SAM9X60

[Claudiu.Beznea]

From: Claudiu Beznea 

Hi,

This series adds support for drive strength and slew rate support for
Microchip SAMX60's pin controller. For drive strenght we could have 2
values: low, high. For slew rate we could have 2 values: enable, disabled.

Thank you,
Claudiu Beznea

Claudiu Beznea (4):
  pinctrl: at91: add option to use drive strength bits
  pinctrl: at91: add drive strength support for SAM9X60
  pinctrl: at91: add compatibles for SAM9X60 pin controller
  pinctrl: at91: add slewrate support for SAM9X60

 drivers/pinctrl/pinctrl-at91.c | 84 +++---
 include/dt-bindings/pinctrl/at91.h |  4 ++
 2 files changed, 83 insertions(+), 5 deletions(-)

-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 1/4] pinctrl: at91: add option to use drive strength bits

[Claudiu.Beznea]

From: Claudiu Beznea 

SAM9X60 uses high and low drive strengths. To implement this, in
at91_pinctrl_mux_ops::set_drivestrength we need bit numbers of
drive strengths (1 for low, 2 for high), thus change the code to
allow the usage of drive strength bit numbers.

Signed-off-by: Claudiu Beznea 
---
 drivers/pinctrl/pinctrl-at91.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 4bdad62dc906..8997732376e4 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -50,10 +50,15 @@ struct at91_pinctrl_priv {
  * DRIVE_STRENGTH_DEFAULT is just a placeholder to avoid changing the drive
  * strength when there is no dt config for it.
  */
-#define DRIVE_STRENGTH_DEFAULT (0 << DRIVE_STRENGTH_SHIFT)
-#define DRIVE_STRENGTH_LOW (1 << DRIVE_STRENGTH_SHIFT)
-#define DRIVE_STRENGTH_MED (2 << DRIVE_STRENGTH_SHIFT)
-#define DRIVE_STRENGTH_HI  (3 << DRIVE_STRENGTH_SHIFT)
+enum drive_strength_bit {
+   DRIVE_STRENGTH_BIT_DEF,
+   DRIVE_STRENGTH_BIT_LOW,
+   DRIVE_STRENGTH_BIT_MED,
+   DRIVE_STRENGTH_BIT_HI,
+};
+
+#define DRIVE_STRENGTH_BIT_MSK(name)   (DRIVE_STRENGTH_BIT_##name << \
+DRIVE_STRENGTH_SHIFT)
 
 enum at91_mux {
AT91_MUX_GPIO = 0,
@@ -238,7 +243,7 @@ static void at91_mux_sam9x5_set_drivestrength(struct 
at91_port *pio,
 
/* strength is inverse on SAM9x5s with our defines
 * 0 = hi, 1 = med, 2 = low, 3 = rsvd */
-   setting = DRIVE_STRENGTH_HI - setting;
+   setting = DRIVE_STRENGTH_BIT_MSK(HI) - setting;
 
set_drive_strength(reg, pin, setting);
 }
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot