| Message ID | 20221208114515.35179-3-claudiu.beznea@microchip.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | clk: at91: mark ddr clocks as critical | expand |
Hi, Krzysztof, On 08.12.2022 13:45, Claudiu Beznea wrote: > Driver does only clock request + enable for DDR clocks. DDR clocks are > enabled by bootloader and need to stay that way in Linux. To avoid having > these clocks disabled by clock subsystem in case there are no Linux > consumers for them the clocks were marked as critical in clock drivers. > With this, there is no need to have a separate driver that only does > clock request + enable. > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> As this patch depends on patch 1/3 from this series, can I have your ack for it to take it though clock tree? Thank you, Claudiu > --- > drivers/memory/Makefile | 1 - > drivers/memory/atmel-sdramc.c | 76 ----------------------------------- > 2 files changed, 77 deletions(-) > delete mode 100644 drivers/memory/atmel-sdramc.c > > diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile > index e148f636c082..d1836be010ec 100644 > --- a/drivers/memory/Makefile > +++ b/drivers/memory/Makefile > @@ -8,7 +8,6 @@ ifeq ($(CONFIG_DDR),y) > obj-$(CONFIG_OF) += of_memory.o > endif > obj-$(CONFIG_ARM_PL172_MPMC) += pl172.o > -obj-$(CONFIG_ATMEL_SDRAMC) += atmel-sdramc.o > obj-$(CONFIG_ATMEL_EBI) += atmel-ebi.o > obj-$(CONFIG_BRCMSTB_DPFE) += brcmstb_dpfe.o > obj-$(CONFIG_BRCMSTB_MEMC) += brcmstb_memc.o > diff --git a/drivers/memory/atmel-sdramc.c b/drivers/memory/atmel-sdramc.c > deleted file mode 100644 > index 9c49d00c2a96..000000000000 > --- a/drivers/memory/atmel-sdramc.c > +++ /dev/null > @@ -1,76 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0-only > -/* > - * Atmel (Multi-port DDR-)SDRAM Controller driver > - * > - * Author: Alexandre Belloni <alexandre.belloni@free-electrons.com> > - * > - * Copyright (C) 2014 Atmel > - */ > - > -#include <linux/clk.h> > -#include <linux/err.h> > -#include <linux/kernel.h> > -#include <linux/init.h> > -#include <linux/of_platform.h> > -#include <linux/platform_device.h> > - > -struct at91_ramc_caps { > - bool has_ddrck; > - bool has_mpddr_clk; > -}; > - > -static const struct at91_ramc_caps at91rm9200_caps = { }; > - > -static const struct at91_ramc_caps at91sam9g45_caps = { > - .has_ddrck = 1, > - .has_mpddr_clk = 0, > -}; > - > -static const struct at91_ramc_caps sama5d3_caps = { > - .has_ddrck = 1, > - .has_mpddr_clk = 1, > -}; > - > -static const struct of_device_id atmel_ramc_of_match[] = { > - { .compatible = "atmel,at91rm9200-sdramc", .data = &at91rm9200_caps, }, > - { .compatible = "atmel,at91sam9260-sdramc", .data = &at91rm9200_caps, }, > - { .compatible = "atmel,at91sam9g45-ddramc", .data = &at91sam9g45_caps, }, > - { .compatible = "atmel,sama5d3-ddramc", .data = &sama5d3_caps, }, > - {}, > -}; > - > -static int atmel_ramc_probe(struct platform_device *pdev) > -{ > - const struct at91_ramc_caps *caps; > - struct clk *clk; > - > - caps = of_device_get_match_data(&pdev->dev); > - > - if (caps->has_ddrck) { > - clk = devm_clk_get(&pdev->dev, "ddrck"); > - if (IS_ERR(clk)) > - return PTR_ERR(clk); > - clk_prepare_enable(clk); > - } > - > - if (caps->has_mpddr_clk) { > - clk = devm_clk_get(&pdev->dev, "mpddr"); > - if (IS_ERR(clk)) { > - pr_err("AT91 RAMC: couldn't get mpddr clock\n"); > - return PTR_ERR(clk); > - } > - clk_prepare_enable(clk); > - } > - > - return 0; > -} > - > -static struct platform_driver atmel_ramc_driver = { > - .probe = atmel_ramc_probe, > - .driver = { > - .name = "atmel-ramc", > - .of_match_table = atmel_ramc_of_match, > - }, > -}; > - > -builtin_platform_driver(atmel_ramc_driver);
On 03/01/2023 11:00, Claudiu.Beznea@microchip.com wrote: > Hi, Krzysztof, > > On 08.12.2022 13:45, Claudiu Beznea wrote: >> Driver does only clock request + enable for DDR clocks. DDR clocks are >> enabled by bootloader and need to stay that way in Linux. To avoid having >> these clocks disabled by clock subsystem in case there are no Linux >> consumers for them the clocks were marked as critical in clock drivers. >> With this, there is no need to have a separate driver that only does >> clock request + enable. >> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > > As this patch depends on patch 1/3 from this series, can I have your ack > for it to take it though clock tree? Uh, why does it depend? I understood the changset is bisectable and removal of unneeded driver will happen later. Otherwise it is not bisectable... Best regards, Krzysztof
On 03.01.2023 12:06, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 03/01/2023 11:00, Claudiu.Beznea@microchip.com wrote: >> Hi, Krzysztof, >> >> On 08.12.2022 13:45, Claudiu Beznea wrote: >>> Driver does only clock request + enable for DDR clocks. DDR clocks are >>> enabled by bootloader and need to stay that way in Linux. To avoid having >>> these clocks disabled by clock subsystem in case there are no Linux >>> consumers for them the clocks were marked as critical in clock drivers. >>> With this, there is no need to have a separate driver that only does >>> clock request + enable. >>> >>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >> >> As this patch depends on patch 1/3 from this series, can I have your ack >> for it to take it though clock tree? > > Uh, why does it depend? I understood the changset is bisectable and > removal of unneeded driver will happen later. Otherwise it is not > bisectable... AT91 devices will fail to boot if this patch is applied and 1/3 is not there. This is because clock framework will disable DDR clocks because there will be no consumer for them. > > Best regards, > Krzysztof >
On 03/01/2023 12:18, Claudiu.Beznea@microchip.com wrote: > On 03.01.2023 12:06, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On 03/01/2023 11:00, Claudiu.Beznea@microchip.com wrote: >>> Hi, Krzysztof, >>> >>> On 08.12.2022 13:45, Claudiu Beznea wrote: >>>> Driver does only clock request + enable for DDR clocks. DDR clocks are >>>> enabled by bootloader and need to stay that way in Linux. To avoid having >>>> these clocks disabled by clock subsystem in case there are no Linux >>>> consumers for them the clocks were marked as critical in clock drivers. >>>> With this, there is no need to have a separate driver that only does >>>> clock request + enable. >>>> >>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >>> >>> As this patch depends on patch 1/3 from this series, can I have your ack >>> for it to take it though clock tree? >> >> Uh, why does it depend? I understood the changset is bisectable and >> removal of unneeded driver will happen later. Otherwise it is not >> bisectable... > > AT91 devices will fail to boot if this patch is applied and 1/3 is not > there. This is because clock framework will disable DDR clocks because > there will be no consumer for them. This I understand, but why do you need this patch to be able to apply 1/3? This is problem of bisectability. Not the other way. Best regards, Krzysztof
On 03.01.2023 14:00, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 03/01/2023 12:18, Claudiu.Beznea@microchip.com wrote: >> On 03.01.2023 12:06, Krzysztof Kozlowski wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> On 03/01/2023 11:00, Claudiu.Beznea@microchip.com wrote: >>>> Hi, Krzysztof, >>>> >>>> On 08.12.2022 13:45, Claudiu Beznea wrote: >>>>> Driver does only clock request + enable for DDR clocks. DDR clocks are >>>>> enabled by bootloader and need to stay that way in Linux. To avoid having >>>>> these clocks disabled by clock subsystem in case there are no Linux >>>>> consumers for them the clocks were marked as critical in clock drivers. >>>>> With this, there is no need to have a separate driver that only does >>>>> clock request + enable. >>>>> >>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >>>> >>>> As this patch depends on patch 1/3 from this series, can I have your ack >>>> for it to take it though clock tree? >>> >>> Uh, why does it depend? I understood the changset is bisectable and >>> removal of unneeded driver will happen later. Otherwise it is not >>> bisectable... >> >> AT91 devices will fail to boot if this patch is applied and 1/3 is not >> there. This is because clock framework will disable DDR clocks because >> there will be no consumer for them. > > This I understand, but why do you need this patch to be able to apply > 1/3? To avoid having AT91 devices failing to boot in case your tree (containing this patch) is merged first. > This is problem of bisectability. Not the other way. > > Best regards, > Krzysztof >
On 03/01/2023 13:45, Claudiu.Beznea@microchip.com wrote: >>>> Uh, why does it depend? I understood the changset is bisectable and >>>> removal of unneeded driver will happen later. Otherwise it is not >>>> bisectable... >>> >>> AT91 devices will fail to boot if this patch is applied and 1/3 is not >>> there. This is because clock framework will disable DDR clocks because >>> there will be no consumer for them. >> >> This I understand, but why do you need this patch to be able to apply >> 1/3? > > To avoid having AT91 devices failing to boot in case your tree (containing > this patch) is merged first. But this patch is not going to be merged first. It will wait one more cycle, so the dependency is there. If you need to make it in one cycle for some reason, then I would need stable tag with the clock patch. Best regards, Krzysztof
On 03.01.2023 15:17, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 03/01/2023 13:45, Claudiu.Beznea@microchip.com wrote: > >>>>> Uh, why does it depend? I understood the changset is bisectable and >>>>> removal of unneeded driver will happen later. Otherwise it is not >>>>> bisectable... >>>> >>>> AT91 devices will fail to boot if this patch is applied and 1/3 is not >>>> there. This is because clock framework will disable DDR clocks because >>>> there will be no consumer for them. >>> >>> This I understand, but why do you need this patch to be able to apply >>> 1/3? >> >> To avoid having AT91 devices failing to boot in case your tree (containing >> this patch) is merged first. > > But this patch is not going to be merged first. It will wait one more > cycle, so the dependency is there. OK. All good this way. Thank you, Claudiu > > If you need to make it in one cycle for some reason, then I would need > stable tag with the clock patch. > > Best regards, > Krzysztof >
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile index e148f636c082..d1836be010ec 100644 --- a/drivers/memory/Makefile +++ b/drivers/memory/Makefile @@ -8,7 +8,6 @@ ifeq ($(CONFIG_DDR),y) obj-$(CONFIG_OF) += of_memory.o endif obj-$(CONFIG_ARM_PL172_MPMC) += pl172.o -obj-$(CONFIG_ATMEL_SDRAMC) += atmel-sdramc.o obj-$(CONFIG_ATMEL_EBI) += atmel-ebi.o obj-$(CONFIG_BRCMSTB_DPFE) += brcmstb_dpfe.o obj-$(CONFIG_BRCMSTB_MEMC) += brcmstb_memc.o diff --git a/drivers/memory/atmel-sdramc.c b/drivers/memory/atmel-sdramc.c deleted file mode 100644 index 9c49d00c2a96..000000000000 --- a/drivers/memory/atmel-sdramc.c +++ /dev/null @@ -1,76 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only -/* - * Atmel (Multi-port DDR-)SDRAM Controller driver - * - * Author: Alexandre Belloni <alexandre.belloni@free-electrons.com> - * - * Copyright (C) 2014 Atmel - */ - -#include <linux/clk.h> -#include <linux/err.h> -#include <linux/kernel.h> -#include <linux/init.h> -#include <linux/of_platform.h> -#include <linux/platform_device.h> - -struct at91_ramc_caps { - bool has_ddrck; - bool has_mpddr_clk; -}; - -static const struct at91_ramc_caps at91rm9200_caps = { }; - -static const struct at91_ramc_caps at91sam9g45_caps = { - .has_ddrck = 1, - .has_mpddr_clk = 0, -}; - -static const struct at91_ramc_caps sama5d3_caps = { - .has_ddrck = 1, - .has_mpddr_clk = 1, -}; - -static const struct of_device_id atmel_ramc_of_match[] = { - { .compatible = "atmel,at91rm9200-sdramc", .data = &at91rm9200_caps, }, - { .compatible = "atmel,at91sam9260-sdramc", .data = &at91rm9200_caps, }, - { .compatible = "atmel,at91sam9g45-ddramc", .data = &at91sam9g45_caps, }, - { .compatible = "atmel,sama5d3-ddramc", .data = &sama5d3_caps, }, - {}, -}; - -static int atmel_ramc_probe(struct platform_device *pdev) -{ - const struct at91_ramc_caps *caps; - struct clk *clk; - - caps = of_device_get_match_data(&pdev->dev); - - if (caps->has_ddrck) { - clk = devm_clk_get(&pdev->dev, "ddrck"); - if (IS_ERR(clk)) - return PTR_ERR(clk); - clk_prepare_enable(clk); - } - - if (caps->has_mpddr_clk) { - clk = devm_clk_get(&pdev->dev, "mpddr"); - if (IS_ERR(clk)) { - pr_err("AT91 RAMC: couldn't get mpddr clock\n"); - return PTR_ERR(clk); - } - clk_prepare_enable(clk); - } - - return 0; -} - -static struct platform_driver atmel_ramc_driver = { - .probe = atmel_ramc_probe, - .driver = { - .name = "atmel-ramc", - .of_match_table = atmel_ramc_of_match, - }, -}; - -builtin_platform_driver(atmel_ramc_driver);
Driver does only clock request + enable for DDR clocks. DDR clocks are enabled by bootloader and need to stay that way in Linux. To avoid having these clocks disabled by clock subsystem in case there are no Linux consumers for them the clocks were marked as critical in clock drivers. With this, there is no need to have a separate driver that only does clock request + enable. Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> --- drivers/memory/Makefile | 1 - drivers/memory/atmel-sdramc.c | 76 ----------------------------------- 2 files changed, 77 deletions(-) delete mode 100644 drivers/memory/atmel-sdramc.c