diff mbox series

[RFC,07/18] clk: bcm2835: Add BCM2838_CLOCK_EMMC2 support

Message ID 1563393026-17118-8-git-send-email-wahrenst@gmx.net (mailing list archive)
State New, archived
Headers show
Series ARM: Add minimal Raspberry Pi 4 support | expand

Commit Message

Stefan Wahren July 17, 2019, 7:50 p.m. UTC
The new BCM2838 supports an additional clock for the emmc2 block.
So add a new compatible to register this clock only for BCM2838.

Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
---
 drivers/clk/bcm/clk-bcm2835.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

--
2.7.4

Comments

Matthias Brugger July 18, 2019, 8:47 a.m. UTC | #1
On 17/07/2019 21:50, Stefan Wahren wrote:
> The new BCM2838 supports an additional clock for the emmc2 block.
> So add a new compatible to register this clock only for BCM2838.
> 
> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
> ---
>  drivers/clk/bcm/clk-bcm2835.c | 33 +++++++++++++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 867ae3c..5fe4695 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -31,7 +31,8 @@
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> -#include <linux/of.h>
> +#include <linux/of_device.h>
> +
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <dt-bindings/clock/bcm2835.h>
> @@ -114,6 +115,8 @@
>  #define CM_AVEODIV		0x1bc
>  #define CM_EMMCCTL		0x1c0
>  #define CM_EMMCDIV		0x1c4
> +#define CM_EMMC2CTL		0x1d0
> +#define CM_EMMC2DIV		0x1d4
> 
>  /* General bits for the CM_*CTL regs */
>  # define CM_ENABLE			BIT(4)
> @@ -1950,6 +1953,15 @@ static const struct bcm2835_clk_desc clk_desc_array[] = {
>  		.frac_bits = 8,
>  		.tcnt_mux = 39),
> 
> +	/* EMMC2 clock (only available for BCM2838) */
> +	[BCM2838_CLOCK_EMMC2]	= REGISTER_PER_CLK(
> +		.name = "emmc2",
> +		.ctl_reg = CM_EMMC2CTL,
> +		.div_reg = CM_EMMC2DIV,
> +		.int_bits = 4,
> +		.frac_bits = 8,
> +		.tcnt_mux = 42),
> +
>  	/* General purpose (GPIO) clocks */
>  	[BCM2835_CLOCK_GP0]	= REGISTER_PER_CLK(
>  		.name = "gp0",
> @@ -2101,6 +2113,14 @@ static int bcm2835_mark_sdc_parent_critical(struct clk *sdc)
>  	return clk_prepare_enable(parent);
>  }
> 
> +bool bcm2835_has_clk(size_t index) {
> +	return index != BCM2838_CLOCK_EMMC2;
> +}
> +
> +bool bcm2838_has_clk(size_t index) {
> +	return true;
> +}
> +
>  static int bcm2835_clk_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -2109,9 +2129,14 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	const struct bcm2835_clk_desc *desc;
>  	const size_t asize = ARRAY_SIZE(clk_desc_array);
> +	bool (*has_clk_func)(size_t);
>  	size_t i;
>  	int ret;
> 
> +	has_clk_func = of_device_get_match_data(&pdev->dev);
> +	if (!has_clk_func)
> +		return -ENODEV;
> +
>  	cprman = devm_kzalloc(dev,
>  			      struct_size(cprman, onecell.hws, asize),
>  			      GFP_KERNEL);
> @@ -2146,6 +2171,9 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>  	hws = cprman->onecell.hws;
> 
>  	for (i = 0; i < asize; i++) {
> +		if (!has_clk_func(i))
> +			continue;
> +

I think that's very hacky. Can't we just create a per SoC list which get's
passed via .data and in probe we iterate through that list and enable the
clocks? That would make clear which clocks are just for bcm2838, basically emmc2.

Regards,
Matthias

>  		desc = &clk_desc_array[i];
>  		if (desc->clk_register && desc->data)
>  			hws[i] = desc->clk_register(cprman, desc->data);
> @@ -2160,7 +2188,8 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>  }
> 
>  static const struct of_device_id bcm2835_clk_of_match[] = {
> -	{ .compatible = "brcm,bcm2835-cprman", },
> +	{ .compatible = "brcm,bcm2835-cprman", .data = bcm2835_has_clk },
> +	{ .compatible = "brcm,bcm2838-cprman", .data = bcm2838_has_clk },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, bcm2835_clk_of_match);
> --
> 2.7.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Florian Fainelli July 18, 2019, 4:51 p.m. UTC | #2
On 7/18/2019 1:47 AM, Matthias Brugger wrote:
> 
> 
> On 17/07/2019 21:50, Stefan Wahren wrote:
>> The new BCM2838 supports an additional clock for the emmc2 block.
>> So add a new compatible to register this clock only for BCM2838.
>>
>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
>> ---
>>  drivers/clk/bcm/clk-bcm2835.c | 33 +++++++++++++++++++++++++++++++--
>>  1 file changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
>> index 867ae3c..5fe4695 100644
>> --- a/drivers/clk/bcm/clk-bcm2835.c
>> +++ b/drivers/clk/bcm/clk-bcm2835.c
>> @@ -31,7 +31,8 @@
>>  #include <linux/delay.h>
>>  #include <linux/io.h>
>>  #include <linux/module.h>
>> -#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +
>>  #include <linux/platform_device.h>
>>  #include <linux/slab.h>
>>  #include <dt-bindings/clock/bcm2835.h>
>> @@ -114,6 +115,8 @@
>>  #define CM_AVEODIV		0x1bc
>>  #define CM_EMMCCTL		0x1c0
>>  #define CM_EMMCDIV		0x1c4
>> +#define CM_EMMC2CTL		0x1d0
>> +#define CM_EMMC2DIV		0x1d4
>>
>>  /* General bits for the CM_*CTL regs */
>>  # define CM_ENABLE			BIT(4)
>> @@ -1950,6 +1953,15 @@ static const struct bcm2835_clk_desc clk_desc_array[] = {
>>  		.frac_bits = 8,
>>  		.tcnt_mux = 39),
>>
>> +	/* EMMC2 clock (only available for BCM2838) */
>> +	[BCM2838_CLOCK_EMMC2]	= REGISTER_PER_CLK(
>> +		.name = "emmc2",
>> +		.ctl_reg = CM_EMMC2CTL,
>> +		.div_reg = CM_EMMC2DIV,
>> +		.int_bits = 4,
>> +		.frac_bits = 8,
>> +		.tcnt_mux = 42),
>> +
>>  	/* General purpose (GPIO) clocks */
>>  	[BCM2835_CLOCK_GP0]	= REGISTER_PER_CLK(
>>  		.name = "gp0",
>> @@ -2101,6 +2113,14 @@ static int bcm2835_mark_sdc_parent_critical(struct clk *sdc)
>>  	return clk_prepare_enable(parent);
>>  }
>>
>> +bool bcm2835_has_clk(size_t index) {
>> +	return index != BCM2838_CLOCK_EMMC2;
>> +}
>> +
>> +bool bcm2838_has_clk(size_t index) {
>> +	return true;
>> +}
>> +
>>  static int bcm2835_clk_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> @@ -2109,9 +2129,14 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>>  	struct resource *res;
>>  	const struct bcm2835_clk_desc *desc;
>>  	const size_t asize = ARRAY_SIZE(clk_desc_array);
>> +	bool (*has_clk_func)(size_t);
>>  	size_t i;
>>  	int ret;
>>
>> +	has_clk_func = of_device_get_match_data(&pdev->dev);
>> +	if (!has_clk_func)
>> +		return -ENODEV;
>> +
>>  	cprman = devm_kzalloc(dev,
>>  			      struct_size(cprman, onecell.hws, asize),
>>  			      GFP_KERNEL);
>> @@ -2146,6 +2171,9 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>>  	hws = cprman->onecell.hws;
>>
>>  	for (i = 0; i < asize; i++) {
>> +		if (!has_clk_func(i))
>> +			continue;
>> +
> 
> I think that's very hacky. Can't we just create a per SoC list which get's
> passed via .data and in probe we iterate through that list and enable the
> clocks? That would make clear which clocks are just for bcm2838, basically emmc2.

Or within the structure, add a flag that indicates whether the clock is
available or not for a given chip? That would avoid having to introduce
bcm283x_has_clk() functions that needs to maintain a possibly growing
list of clocks. You would still have to check the flag though.

The trade-off here is a large table (larger than necessary) versus more
run time checks.

We could consider having a base table that gets copied into a bcm2838
specific table, and overlay that base structure with what's specific
with bcm2838, is that you have in mind?
Eric Anholt July 18, 2019, 6:37 p.m. UTC | #3
Florian Fainelli <f.fainelli@gmail.com> writes:

> On 7/18/2019 1:47 AM, Matthias Brugger wrote:
>> 
>> 
>> On 17/07/2019 21:50, Stefan Wahren wrote:
>>> The new BCM2838 supports an additional clock for the emmc2 block.
>>> So add a new compatible to register this clock only for BCM2838.
>>>
>>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
>>> ---
>>>  drivers/clk/bcm/clk-bcm2835.c | 33 +++++++++++++++++++++++++++++++--
>>>  1 file changed, 31 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
>>> index 867ae3c..5fe4695 100644
>>> --- a/drivers/clk/bcm/clk-bcm2835.c
>>> +++ b/drivers/clk/bcm/clk-bcm2835.c
>>> @@ -31,7 +31,8 @@
>>>  #include <linux/delay.h>
>>>  #include <linux/io.h>
>>>  #include <linux/module.h>
>>> -#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>> +
>>>  #include <linux/platform_device.h>
>>>  #include <linux/slab.h>
>>>  #include <dt-bindings/clock/bcm2835.h>
>>> @@ -114,6 +115,8 @@
>>>  #define CM_AVEODIV		0x1bc
>>>  #define CM_EMMCCTL		0x1c0
>>>  #define CM_EMMCDIV		0x1c4
>>> +#define CM_EMMC2CTL		0x1d0
>>> +#define CM_EMMC2DIV		0x1d4
>>>
>>>  /* General bits for the CM_*CTL regs */
>>>  # define CM_ENABLE			BIT(4)
>>> @@ -1950,6 +1953,15 @@ static const struct bcm2835_clk_desc clk_desc_array[] = {
>>>  		.frac_bits = 8,
>>>  		.tcnt_mux = 39),
>>>
>>> +	/* EMMC2 clock (only available for BCM2838) */
>>> +	[BCM2838_CLOCK_EMMC2]	= REGISTER_PER_CLK(
>>> +		.name = "emmc2",
>>> +		.ctl_reg = CM_EMMC2CTL,
>>> +		.div_reg = CM_EMMC2DIV,
>>> +		.int_bits = 4,
>>> +		.frac_bits = 8,
>>> +		.tcnt_mux = 42),
>>> +
>>>  	/* General purpose (GPIO) clocks */
>>>  	[BCM2835_CLOCK_GP0]	= REGISTER_PER_CLK(
>>>  		.name = "gp0",
>>> @@ -2101,6 +2113,14 @@ static int bcm2835_mark_sdc_parent_critical(struct clk *sdc)
>>>  	return clk_prepare_enable(parent);
>>>  }
>>>
>>> +bool bcm2835_has_clk(size_t index) {
>>> +	return index != BCM2838_CLOCK_EMMC2;
>>> +}
>>> +
>>> +bool bcm2838_has_clk(size_t index) {
>>> +	return true;
>>> +}
>>> +
>>>  static int bcm2835_clk_probe(struct platform_device *pdev)
>>>  {
>>>  	struct device *dev = &pdev->dev;
>>> @@ -2109,9 +2129,14 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>>>  	struct resource *res;
>>>  	const struct bcm2835_clk_desc *desc;
>>>  	const size_t asize = ARRAY_SIZE(clk_desc_array);
>>> +	bool (*has_clk_func)(size_t);
>>>  	size_t i;
>>>  	int ret;
>>>
>>> +	has_clk_func = of_device_get_match_data(&pdev->dev);
>>> +	if (!has_clk_func)
>>> +		return -ENODEV;
>>> +
>>>  	cprman = devm_kzalloc(dev,
>>>  			      struct_size(cprman, onecell.hws, asize),
>>>  			      GFP_KERNEL);
>>> @@ -2146,6 +2171,9 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>>>  	hws = cprman->onecell.hws;
>>>
>>>  	for (i = 0; i < asize; i++) {
>>> +		if (!has_clk_func(i))
>>> +			continue;
>>> +
>> 
>> I think that's very hacky. Can't we just create a per SoC list which get's
>> passed via .data and in probe we iterate through that list and enable the
>> clocks? That would make clear which clocks are just for bcm2838, basically emmc2.
>
> Or within the structure, add a flag that indicates whether the clock is
> available or not for a given chip? That would avoid having to introduce
> bcm283x_has_clk() functions that needs to maintain a possibly growing
> list of clocks. You would still have to check the flag though.

I think this is a good solution.
Stefan Wahren July 19, 2019, 4:40 p.m. UTC | #4
Hi,

Am 18.07.19 um 20:37 schrieb Eric Anholt:
> Florian Fainelli <f.fainelli@gmail.com> writes:
>
>> On 7/18/2019 1:47 AM, Matthias Brugger wrote:
>>>
>>> On 17/07/2019 21:50, Stefan Wahren wrote:
>>>> The new BCM2838 supports an additional clock for the emmc2 block.
>>>> So add a new compatible to register this clock only for BCM2838.
>>>>
>>>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
>>>> ---
>>>>  drivers/clk/bcm/clk-bcm2835.c | 33 +++++++++++++++++++++++++++++++--
>>>>  1 file changed, 31 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
>>>> index 867ae3c..5fe4695 100644
>>>> --- a/drivers/clk/bcm/clk-bcm2835.c
>>>> +++ b/drivers/clk/bcm/clk-bcm2835.c
>>>> @@ -31,7 +31,8 @@
>>>>  #include <linux/delay.h>
>>>>  #include <linux/io.h>
>>>>  #include <linux/module.h>
>>>> -#include <linux/of.h>
>>>> +#include <linux/of_device.h>
>>>> +
>>>>  #include <linux/platform_device.h>
>>>>  #include <linux/slab.h>
>>>>  #include <dt-bindings/clock/bcm2835.h>
>>>> @@ -114,6 +115,8 @@
>>>>  #define CM_AVEODIV		0x1bc
>>>>  #define CM_EMMCCTL		0x1c0
>>>>  #define CM_EMMCDIV		0x1c4
>>>> +#define CM_EMMC2CTL		0x1d0
>>>> +#define CM_EMMC2DIV		0x1d4
>>>>
>>>>  /* General bits for the CM_*CTL regs */
>>>>  # define CM_ENABLE			BIT(4)
>>>> @@ -1950,6 +1953,15 @@ static const struct bcm2835_clk_desc clk_desc_array[] = {
>>>>  		.frac_bits = 8,
>>>>  		.tcnt_mux = 39),
>>>>
>>>> +	/* EMMC2 clock (only available for BCM2838) */
>>>> +	[BCM2838_CLOCK_EMMC2]	= REGISTER_PER_CLK(
>>>> +		.name = "emmc2",
>>>> +		.ctl_reg = CM_EMMC2CTL,
>>>> +		.div_reg = CM_EMMC2DIV,
>>>> +		.int_bits = 4,
>>>> +		.frac_bits = 8,
>>>> +		.tcnt_mux = 42),
>>>> +
>>>>  	/* General purpose (GPIO) clocks */
>>>>  	[BCM2835_CLOCK_GP0]	= REGISTER_PER_CLK(
>>>>  		.name = "gp0",
>>>> @@ -2101,6 +2113,14 @@ static int bcm2835_mark_sdc_parent_critical(struct clk *sdc)
>>>>  	return clk_prepare_enable(parent);
>>>>  }
>>>>
>>>> +bool bcm2835_has_clk(size_t index) {
>>>> +	return index != BCM2838_CLOCK_EMMC2;
>>>> +}
>>>> +
>>>> +bool bcm2838_has_clk(size_t index) {
>>>> +	return true;
>>>> +}
>>>> +
>>>>  static int bcm2835_clk_probe(struct platform_device *pdev)
>>>>  {
>>>>  	struct device *dev = &pdev->dev;
>>>> @@ -2109,9 +2129,14 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>>>>  	struct resource *res;
>>>>  	const struct bcm2835_clk_desc *desc;
>>>>  	const size_t asize = ARRAY_SIZE(clk_desc_array);
>>>> +	bool (*has_clk_func)(size_t);
>>>>  	size_t i;
>>>>  	int ret;
>>>>
>>>> +	has_clk_func = of_device_get_match_data(&pdev->dev);
>>>> +	if (!has_clk_func)
>>>> +		return -ENODEV;
>>>> +
>>>>  	cprman = devm_kzalloc(dev,
>>>>  			      struct_size(cprman, onecell.hws, asize),
>>>>  			      GFP_KERNEL);
>>>> @@ -2146,6 +2171,9 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>>>>  	hws = cprman->onecell.hws;
>>>>
>>>>  	for (i = 0; i < asize; i++) {
>>>> +		if (!has_clk_func(i))
>>>> +			continue;
>>>> +
>>> I think that's very hacky. Can't we just create a per SoC list which get's
>>> passed via .data and in probe we iterate through that list and enable the
>>> clocks? That would make clear which clocks are just for bcm2838, basically emmc2.
>> Or within the structure, add a flag that indicates whether the clock is
>> available or not for a given chip? That would avoid having to introduce
>> bcm283x_has_clk() functions that needs to maintain a possibly growing
>> list of clocks. You would still have to check the flag though.
> I think this is a good solution.

i only want to make sure that i get it right:

- add a new member supported (e.g. unsigned int) into struct
bcm2835_clk_desc
- bit 1 should be BCM2835 and 2 should be BCM2711 (so BCM7211 could use
3 and so ...)
- during probing we should check the bit against the SoC bit before
registration

Stefan
Eric Anholt July 19, 2019, 7:49 p.m. UTC | #5
Stefan Wahren <wahrenst@gmx.net> writes:

> Hi,
>
> Am 18.07.19 um 20:37 schrieb Eric Anholt:
>> Florian Fainelli <f.fainelli@gmail.com> writes:
>>
>>> On 7/18/2019 1:47 AM, Matthias Brugger wrote:
>>>>
>>>> On 17/07/2019 21:50, Stefan Wahren wrote:
>>>>> The new BCM2838 supports an additional clock for the emmc2 block.
>>>>> So add a new compatible to register this clock only for BCM2838.
>>>>>
>>>>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
>>>>> ---
>>>>>  drivers/clk/bcm/clk-bcm2835.c | 33 +++++++++++++++++++++++++++++++--
>>>>>  1 file changed, 31 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
>>>>> index 867ae3c..5fe4695 100644
>>>>> --- a/drivers/clk/bcm/clk-bcm2835.c
>>>>> +++ b/drivers/clk/bcm/clk-bcm2835.c
>>>>> @@ -31,7 +31,8 @@
>>>>>  #include <linux/delay.h>
>>>>>  #include <linux/io.h>
>>>>>  #include <linux/module.h>
>>>>> -#include <linux/of.h>
>>>>> +#include <linux/of_device.h>
>>>>> +
>>>>>  #include <linux/platform_device.h>
>>>>>  #include <linux/slab.h>
>>>>>  #include <dt-bindings/clock/bcm2835.h>
>>>>> @@ -114,6 +115,8 @@
>>>>>  #define CM_AVEODIV		0x1bc
>>>>>  #define CM_EMMCCTL		0x1c0
>>>>>  #define CM_EMMCDIV		0x1c4
>>>>> +#define CM_EMMC2CTL		0x1d0
>>>>> +#define CM_EMMC2DIV		0x1d4
>>>>>
>>>>>  /* General bits for the CM_*CTL regs */
>>>>>  # define CM_ENABLE			BIT(4)
>>>>> @@ -1950,6 +1953,15 @@ static const struct bcm2835_clk_desc clk_desc_array[] = {
>>>>>  		.frac_bits = 8,
>>>>>  		.tcnt_mux = 39),
>>>>>
>>>>> +	/* EMMC2 clock (only available for BCM2838) */
>>>>> +	[BCM2838_CLOCK_EMMC2]	= REGISTER_PER_CLK(
>>>>> +		.name = "emmc2",
>>>>> +		.ctl_reg = CM_EMMC2CTL,
>>>>> +		.div_reg = CM_EMMC2DIV,
>>>>> +		.int_bits = 4,
>>>>> +		.frac_bits = 8,
>>>>> +		.tcnt_mux = 42),
>>>>> +
>>>>>  	/* General purpose (GPIO) clocks */
>>>>>  	[BCM2835_CLOCK_GP0]	= REGISTER_PER_CLK(
>>>>>  		.name = "gp0",
>>>>> @@ -2101,6 +2113,14 @@ static int bcm2835_mark_sdc_parent_critical(struct clk *sdc)
>>>>>  	return clk_prepare_enable(parent);
>>>>>  }
>>>>>
>>>>> +bool bcm2835_has_clk(size_t index) {
>>>>> +	return index != BCM2838_CLOCK_EMMC2;
>>>>> +}
>>>>> +
>>>>> +bool bcm2838_has_clk(size_t index) {
>>>>> +	return true;
>>>>> +}
>>>>> +
>>>>>  static int bcm2835_clk_probe(struct platform_device *pdev)
>>>>>  {
>>>>>  	struct device *dev = &pdev->dev;
>>>>> @@ -2109,9 +2129,14 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>>>>>  	struct resource *res;
>>>>>  	const struct bcm2835_clk_desc *desc;
>>>>>  	const size_t asize = ARRAY_SIZE(clk_desc_array);
>>>>> +	bool (*has_clk_func)(size_t);
>>>>>  	size_t i;
>>>>>  	int ret;
>>>>>
>>>>> +	has_clk_func = of_device_get_match_data(&pdev->dev);
>>>>> +	if (!has_clk_func)
>>>>> +		return -ENODEV;
>>>>> +
>>>>>  	cprman = devm_kzalloc(dev,
>>>>>  			      struct_size(cprman, onecell.hws, asize),
>>>>>  			      GFP_KERNEL);
>>>>> @@ -2146,6 +2171,9 @@ static int bcm2835_clk_probe(struct platform_device *pdev)
>>>>>  	hws = cprman->onecell.hws;
>>>>>
>>>>>  	for (i = 0; i < asize; i++) {
>>>>> +		if (!has_clk_func(i))
>>>>> +			continue;
>>>>> +
>>>> I think that's very hacky. Can't we just create a per SoC list which get's
>>>> passed via .data and in probe we iterate through that list and enable the
>>>> clocks? That would make clear which clocks are just for bcm2838, basically emmc2.
>>> Or within the structure, add a flag that indicates whether the clock is
>>> available or not for a given chip? That would avoid having to introduce
>>> bcm283x_has_clk() functions that needs to maintain a possibly growing
>>> list of clocks. You would still have to check the flag though.
>> I think this is a good solution.
>
> i only want to make sure that i get it right:
>
> - add a new member supported (e.g. unsigned int) into struct
> bcm2835_clk_desc
> - bit 1 should be BCM2835 and 2 should be BCM2711 (so BCM7211 could use
> 3 and so ...)
> - during probing we should check the bit against the SoC bit before
> registration

Sounds good to me.
diff mbox series

Patch

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 867ae3c..5fe4695 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -31,7 +31,8 @@ 
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/module.h>
-#include <linux/of.h>
+#include <linux/of_device.h>
+
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <dt-bindings/clock/bcm2835.h>
@@ -114,6 +115,8 @@ 
 #define CM_AVEODIV		0x1bc
 #define CM_EMMCCTL		0x1c0
 #define CM_EMMCDIV		0x1c4
+#define CM_EMMC2CTL		0x1d0
+#define CM_EMMC2DIV		0x1d4

 /* General bits for the CM_*CTL regs */
 # define CM_ENABLE			BIT(4)
@@ -1950,6 +1953,15 @@  static const struct bcm2835_clk_desc clk_desc_array[] = {
 		.frac_bits = 8,
 		.tcnt_mux = 39),

+	/* EMMC2 clock (only available for BCM2838) */
+	[BCM2838_CLOCK_EMMC2]	= REGISTER_PER_CLK(
+		.name = "emmc2",
+		.ctl_reg = CM_EMMC2CTL,
+		.div_reg = CM_EMMC2DIV,
+		.int_bits = 4,
+		.frac_bits = 8,
+		.tcnt_mux = 42),
+
 	/* General purpose (GPIO) clocks */
 	[BCM2835_CLOCK_GP0]	= REGISTER_PER_CLK(
 		.name = "gp0",
@@ -2101,6 +2113,14 @@  static int bcm2835_mark_sdc_parent_critical(struct clk *sdc)
 	return clk_prepare_enable(parent);
 }

+bool bcm2835_has_clk(size_t index) {
+	return index != BCM2838_CLOCK_EMMC2;
+}
+
+bool bcm2838_has_clk(size_t index) {
+	return true;
+}
+
 static int bcm2835_clk_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -2109,9 +2129,14 @@  static int bcm2835_clk_probe(struct platform_device *pdev)
 	struct resource *res;
 	const struct bcm2835_clk_desc *desc;
 	const size_t asize = ARRAY_SIZE(clk_desc_array);
+	bool (*has_clk_func)(size_t);
 	size_t i;
 	int ret;

+	has_clk_func = of_device_get_match_data(&pdev->dev);
+	if (!has_clk_func)
+		return -ENODEV;
+
 	cprman = devm_kzalloc(dev,
 			      struct_size(cprman, onecell.hws, asize),
 			      GFP_KERNEL);
@@ -2146,6 +2171,9 @@  static int bcm2835_clk_probe(struct platform_device *pdev)
 	hws = cprman->onecell.hws;

 	for (i = 0; i < asize; i++) {
+		if (!has_clk_func(i))
+			continue;
+
 		desc = &clk_desc_array[i];
 		if (desc->clk_register && desc->data)
 			hws[i] = desc->clk_register(cprman, desc->data);
@@ -2160,7 +2188,8 @@  static int bcm2835_clk_probe(struct platform_device *pdev)
 }

 static const struct of_device_id bcm2835_clk_of_match[] = {
-	{ .compatible = "brcm,bcm2835-cprman", },
+	{ .compatible = "brcm,bcm2835-cprman", .data = bcm2835_has_clk },
+	{ .compatible = "brcm,bcm2838-cprman", .data = bcm2838_has_clk },
 	{}
 };
 MODULE_DEVICE_TABLE(of, bcm2835_clk_of_match);