* [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property
@ 2021-05-26 19:33 Andy Shevchenko
2021-05-26 19:33 ` [PATCH v2 2/2] can: mcp251xfd: Fix header block to clarify independence from OF Andy Shevchenko
2021-05-31 8:47 ` [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property Marc Kleine-Budde
0 siblings, 2 replies; 6+ messages in thread
From: Andy Shevchenko @ 2021-05-26 19:33 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can, netdev, linux-kernel
Cc: Manivannan Sadhasivam, Thomas Kopp, Wolfgang Grandegger,
David S. Miller, Jakub Kicinski, Andy Shevchenko
In some configurations, mainly ACPI-based, the clock frequency of the device
is supplied by very well established 'clock-frequency' property. Hence, try
to get it from the property at last if no other providers are available.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: new patch
drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index e0ae00e34c7b..e42f87c3f2ec 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -2856,7 +2856,7 @@ static int mcp251xfd_probe(struct spi_device *spi)
struct gpio_desc *rx_int;
struct regulator *reg_vdd, *reg_xceiver;
struct clk *clk;
- u32 freq;
+ u32 freq, rate;
int err;
if (!spi->irq)
@@ -2883,11 +2883,16 @@ static int mcp251xfd_probe(struct spi_device *spi)
return dev_err_probe(&spi->dev, PTR_ERR(reg_xceiver),
"Failed to get Transceiver regulator!\n");
- clk = devm_clk_get(&spi->dev, NULL);
+ /* Always ask for fixed clock rate from a property. */
+ device_property_read_u32(&spi->dev, "clock-frequency", &rate);
+
+ clk = devm_clk_get_optional(&spi->dev, NULL);
if (IS_ERR(clk))
return dev_err_probe(&spi->dev, PTR_ERR(clk),
"Failed to get Oscillator (clock)!\n");
freq = clk_get_rate(clk);
+ if (freq == 0)
+ freq = rate;
/* Sanity check */
if (freq < MCP251XFD_SYSCLOCK_HZ_MIN ||
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] can: mcp251xfd: Fix header block to clarify independence from OF
2021-05-26 19:33 [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property Andy Shevchenko
@ 2021-05-26 19:33 ` Andy Shevchenko
2021-05-31 8:47 ` [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property Marc Kleine-Budde
1 sibling, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2021-05-26 19:33 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can, netdev, linux-kernel
Cc: Manivannan Sadhasivam, Thomas Kopp, Wolfgang Grandegger,
David S. Miller, Jakub Kicinski, Andy Shevchenko
The driver is neither dependent on OF, nor it requires any OF headers.
Fix header block to clarify independence from OF.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: included property.h
drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
index e42f87c3f2ec..e919f7e4d566 100644
--- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
+++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
@@ -15,10 +15,10 @@
#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/device.h>
+#include <linux/mod_devicetable.h>
#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
#include <linux/pm_runtime.h>
+#include <linux/property.h>
#include <asm/unaligned.h>
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property
2021-05-26 19:33 [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property Andy Shevchenko
2021-05-26 19:33 ` [PATCH v2 2/2] can: mcp251xfd: Fix header block to clarify independence from OF Andy Shevchenko
@ 2021-05-31 8:47 ` Marc Kleine-Budde
2021-05-31 9:58 ` Andy Shevchenko
1 sibling, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2021-05-31 8:47 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-can, netdev, linux-kernel, Manivannan Sadhasivam,
Thomas Kopp, Wolfgang Grandegger, David S. Miller,
Jakub Kicinski
[-- Attachment #1: Type: text/plain, Size: 2175 bytes --]
On 26.05.2021 22:33:26, Andy Shevchenko wrote:
> In some configurations, mainly ACPI-based, the clock frequency of the device
> is supplied by very well established 'clock-frequency' property. Hence, try
> to get it from the property at last if no other providers are available.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: new patch
> drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> index e0ae00e34c7b..e42f87c3f2ec 100644
> --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-core.c
> @@ -2856,7 +2856,7 @@ static int mcp251xfd_probe(struct spi_device *spi)
> struct gpio_desc *rx_int;
> struct regulator *reg_vdd, *reg_xceiver;
> struct clk *clk;
> - u32 freq;
> + u32 freq, rate;
> int err;
>
> if (!spi->irq)
> @@ -2883,11 +2883,16 @@ static int mcp251xfd_probe(struct spi_device *spi)
> return dev_err_probe(&spi->dev, PTR_ERR(reg_xceiver),
> "Failed to get Transceiver regulator!\n");
>
> - clk = devm_clk_get(&spi->dev, NULL);
> + /* Always ask for fixed clock rate from a property. */
> + device_property_read_u32(&spi->dev, "clock-frequency", &rate);
what about error handling....?
> +
> + clk = devm_clk_get_optional(&spi->dev, NULL);
> if (IS_ERR(clk))
> return dev_err_probe(&spi->dev, PTR_ERR(clk),
> "Failed to get Oscillator (clock)!\n");
> freq = clk_get_rate(clk);
> + if (freq == 0)
> + freq = rate;
... this means we don't fail if there is neither a clk nor a
clock-frequency property. I've send a v3 to fix this.
>
> /* Sanity check */
> if (freq < MCP251XFD_SYSCLOCK_HZ_MIN ||
> --
> 2.30.2
>
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property
2021-05-31 8:47 ` [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property Marc Kleine-Budde
@ 2021-05-31 9:58 ` Andy Shevchenko
2021-05-31 10:03 ` Andy Shevchenko
2021-05-31 10:03 ` Marc Kleine-Budde
0 siblings, 2 replies; 6+ messages in thread
From: Andy Shevchenko @ 2021-05-31 9:58 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: linux-can, netdev, linux-kernel, Manivannan Sadhasivam,
Thomas Kopp, Wolfgang Grandegger, David S. Miller,
Jakub Kicinski
On Mon, May 31, 2021 at 10:47:20AM +0200, Marc Kleine-Budde wrote:
> On 26.05.2021 22:33:26, Andy Shevchenko wrote:
> > In some configurations, mainly ACPI-based, the clock frequency of the device
> > is supplied by very well established 'clock-frequency' property. Hence, try
> > to get it from the property at last if no other providers are available.
> > return dev_err_probe(&spi->dev, PTR_ERR(reg_xceiver),
> > "Failed to get Transceiver regulator!\n");
> >
> > - clk = devm_clk_get(&spi->dev, NULL);
> > + /* Always ask for fixed clock rate from a property. */
> > + device_property_read_u32(&spi->dev, "clock-frequency", &rate);
>
> what about error handling....?
Not needed, but rate should be assigned to 0, which is missed.
> > + clk = devm_clk_get_optional(&spi->dev, NULL);
> > if (IS_ERR(clk))
> > return dev_err_probe(&spi->dev, PTR_ERR(clk),
> > "Failed to get Oscillator (clock)!\n");
> > freq = clk_get_rate(clk);
> > + if (freq == 0)
> > + freq = rate;
>
> ... this means we don't fail if there is neither a clk nor a
> clock-frequency property.
The following will check for it (which is already in the code)
if (freq <= MCP251XFD_SYSCLOCK_HZ_MAX / MCP251XFD_OSC_PLL_MULTIPLIER) {
> I've send a v3 to fix this.
You mean I have to send v3?
Sure!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property
2021-05-31 9:58 ` Andy Shevchenko
@ 2021-05-31 10:03 ` Andy Shevchenko
2021-05-31 10:03 ` Marc Kleine-Budde
1 sibling, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2021-05-31 10:03 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: linux-can, netdev, linux-kernel, Manivannan Sadhasivam,
Thomas Kopp, Wolfgang Grandegger, David S. Miller,
Jakub Kicinski
On Mon, May 31, 2021 at 12:58:29PM +0300, Andy Shevchenko wrote:
> On Mon, May 31, 2021 at 10:47:20AM +0200, Marc Kleine-Budde wrote:
> > On 26.05.2021 22:33:26, Andy Shevchenko wrote:
> > > In some configurations, mainly ACPI-based, the clock frequency of the device
> > > is supplied by very well established 'clock-frequency' property. Hence, try
> > > to get it from the property at last if no other providers are available.
>
> > > return dev_err_probe(&spi->dev, PTR_ERR(reg_xceiver),
> > > "Failed to get Transceiver regulator!\n");
> > >
> > > - clk = devm_clk_get(&spi->dev, NULL);
> > > + /* Always ask for fixed clock rate from a property. */
> > > + device_property_read_u32(&spi->dev, "clock-frequency", &rate);
> >
> > what about error handling....?
>
> Not needed, but rate should be assigned to 0, which is missed.
>
> > > + clk = devm_clk_get_optional(&spi->dev, NULL);
> > > if (IS_ERR(clk))
> > > return dev_err_probe(&spi->dev, PTR_ERR(clk),
> > > "Failed to get Oscillator (clock)!\n");
> > > freq = clk_get_rate(clk);
> > > + if (freq == 0)
> > > + freq = rate;
> >
> > ... this means we don't fail if there is neither a clk nor a
> > clock-frequency property.
>
> The following will check for it (which is already in the code)
>
> if (freq <= MCP251XFD_SYSCLOCK_HZ_MAX / MCP251XFD_OSC_PLL_MULTIPLIER) {
Even before, actually,
/* Sanity check */
if (freq < MCP251XFD_SYSCLOCK_HZ_MIN ||
freq > MCP251XFD_SYSCLOCK_HZ_MAX) {
> > I've send a v3 to fix this.
>
> You mean I have to send v3?
> Sure!
So, I am going to send a v3 with amended commit message and assigning rate
to 0.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property
2021-05-31 9:58 ` Andy Shevchenko
2021-05-31 10:03 ` Andy Shevchenko
@ 2021-05-31 10:03 ` Marc Kleine-Budde
1 sibling, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2021-05-31 10:03 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-can, netdev, linux-kernel, Manivannan Sadhasivam,
Thomas Kopp, Wolfgang Grandegger, David S. Miller,
Jakub Kicinski
[-- Attachment #1: Type: text/plain, Size: 1854 bytes --]
On 31.05.2021 12:58:29, Andy Shevchenko wrote:
> On Mon, May 31, 2021 at 10:47:20AM +0200, Marc Kleine-Budde wrote:
> > On 26.05.2021 22:33:26, Andy Shevchenko wrote:
> > > In some configurations, mainly ACPI-based, the clock frequency of the device
> > > is supplied by very well established 'clock-frequency' property. Hence, try
> > > to get it from the property at last if no other providers are available.
>
> > > return dev_err_probe(&spi->dev, PTR_ERR(reg_xceiver),
> > > "Failed to get Transceiver regulator!\n");
> > >
> > > - clk = devm_clk_get(&spi->dev, NULL);
> > > + /* Always ask for fixed clock rate from a property. */
> > > + device_property_read_u32(&spi->dev, "clock-frequency", &rate);
> >
> > what about error handling....?
>
> Not needed, but rate should be assigned to 0, which is missed.
>
> > > + clk = devm_clk_get_optional(&spi->dev, NULL);
> > > if (IS_ERR(clk))
> > > return dev_err_probe(&spi->dev, PTR_ERR(clk),
> > > "Failed to get Oscillator (clock)!\n");
> > > freq = clk_get_rate(clk);
> > > + if (freq == 0)
> > > + freq = rate;
> >
> > ... this means we don't fail if there is neither a clk nor a
> > clock-frequency property.
>
> The following will check for it (which is already in the code)
>
> if (freq <= MCP251XFD_SYSCLOCK_HZ_MAX / MCP251XFD_OSC_PLL_MULTIPLIER) {
Good point.
> > I've send a v3 to fix this.
>
> You mean I have to send v3?
> Sure!
I have send a v3, see:
https://lore.kernel.org/r/20210531084444.1785397-1-mkl@pengutronix.de
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-31 10:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-26 19:33 [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property Andy Shevchenko
2021-05-26 19:33 ` [PATCH v2 2/2] can: mcp251xfd: Fix header block to clarify independence from OF Andy Shevchenko
2021-05-31 8:47 ` [PATCH v2 1/2] can: mcp251xfd: Try to get crystal clock rate from property Marc Kleine-Budde
2021-05-31 9:58 ` Andy Shevchenko
2021-05-31 10:03 ` Andy Shevchenko
2021-05-31 10:03 ` Marc Kleine-Budde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).