All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: "Duje Mihanović" <duje.mihanovic@skole.hr>
Cc: Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	~postmarketos/upstreaming@lists.sr.ht,
	phone-devel@vger.kernel.org, afaerber@suse.com
Subject: Re: [PATCH 04/10] clk: mmp: Add Marvell PXA1908 clock driver
Date: Mon, 24 Jul 2023 12:05:29 +0300	[thread overview]
Message-ID: <ZL4+2bS3cVGq7q/5@smile.fi.intel.com> (raw)
In-Reply-To: <20230721210042.21535-5-duje.mihanovic@skole.hr>

On Fri, Jul 21, 2023 at 10:37:46PM +0200, Duje Mihanović wrote:
> Add driver for Marvell PXA1908 clock controller blocks. The SoC has
> numerous clock controller blocks, currently supporting APBC, APBCP, MPMU
> and APMU.

...

> +#include <linux/kernel.h>

Try to avoid using this header without real need.
You have missing a ton of the header inclusions, btw.

> +#include <linux/of_address.h>

> +#define APBC_INDEX_TO_OFFSET(n)	((n - 1) * 4)
> +
> +#define APMU_CLK_GATE_CTRL	0x40
> +#define MPMU_UART_PLL		0x14

...

> +static struct mmp_param_fixed_rate_clk fixed_rate_clks[] = {
> +	{PXA1908_CLK_CLK32, "clk32", NULL, 0, 32768},
> +	{PXA1908_CLK_VCTCXO, "vctcxo", NULL, 0, 26000000},

HZ_PER_MHZ (from units.h) here and elsewhere?

> +	{PXA1908_CLK_PLL1_624, "pll1_624", NULL, 0, 624000000},
> +	{PXA1908_CLK_PLL1_416, "pll1_416", NULL, 0, 416000000},
> +	{PXA1908_CLK_PLL1_499, "pll1_499", NULL, 0, 499000000},
> +	{PXA1908_CLK_PLL1_832, "pll1_832", NULL, 0, 832000000},
> +	{PXA1908_CLK_PLL1_1248, "pll1_1248", NULL, 0, 1248000000},
> +};

...

> +static struct mmp_clk_factor_masks uart_factor_masks = {
> +	.factor = 2,

> +	.num_mask = 0x1fff,
> +	.den_mask = 0x1fff,

GENMASK() (provided in bits.h).

> +	.num_shift = 16,
> +	.den_shift = 0,
> +};

...

> +static struct mmp_clk_factor_tbl uart_factor_tbl[] = {
> +	{.num = 8125, .den = 1536},	/* 14.745MHz */
> +};

Can this struct be dropped in favour of struct u32_fract (from math.h)?

...

> +	pxa_unit->apbc_base = of_iomap(np, 0);
> +	if (!pxa_unit->apbc_base) {
> +		pr_err("failed to map apbc registers\n");

Haven't noticed if you are using pr_fmt().

> +		kfree(pxa_unit);
> +		return;
> +	}

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2023-07-24  9:05 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-21 20:37 [PATCH 00/10] Initial Marvell PXA1908 support Duje Mihanović
2023-07-21 20:37 ` Duje Mihanović
2023-07-21 20:37 ` [PATCH 01/10] tty: serial: 8250: Define earlycon for mrvl,mmp-uart Duje Mihanović
2023-07-21 20:37 ` [PATCH 02/10] gpio: pxa: use dynamic allocation of base Duje Mihanović
2023-07-24  9:00   ` Andy Shevchenko
2023-07-21 20:37 ` [PATCH 03/10] gpio: pxa: make pxa_gpio_has_pinctrl return false for MMP_GPIO Duje Mihanović
2023-07-24  9:01   ` Andy Shevchenko
2023-07-21 20:37 ` [PATCH 04/10] clk: mmp: Add Marvell PXA1908 clock driver Duje Mihanović
2023-07-21 20:37   ` Duje Mihanović
2023-07-24  9:05   ` Andy Shevchenko [this message]
2023-07-21 20:37 ` [PATCH 05/10] dt-bindings: clock: Add Marvell PXA1908 clock bindings Duje Mihanović
2023-07-21 20:37   ` Duje Mihanović
2023-07-22  9:21   ` Krzysztof Kozlowski
2023-07-22  9:21     ` Krzysztof Kozlowski
2023-07-21 20:37 ` [PATCH 06/10] dt-bindings: clock: Add documentation for Marvell PXA1908 Duje Mihanović
2023-07-22  9:23   ` Krzysztof Kozlowski
2023-07-21 20:37 ` [PATCH 07/10] arm64: Kconfig.platforms: Add config for Marvell PXA1908 platform Duje Mihanović
2023-07-21 20:37   ` Duje Mihanović
2023-07-21 20:37 ` [PATCH 08/10] arm64: dts: Add DTS for Marvell PXA1908 and samsung,coreprimevelte Duje Mihanović
2023-07-21 20:37   ` Duje Mihanović
2023-07-22  9:25   ` Krzysztof Kozlowski
2023-07-22  9:25     ` Krzysztof Kozlowski
2023-07-21 20:37 ` [PATCH 09/10] dt-bindings: marvell: Document PXA1908 SoC Duje Mihanović
2023-07-21 20:37   ` Duje Mihanović
2023-07-21 22:28   ` Rob Herring
2023-07-21 22:28     ` Rob Herring
2023-07-22  9:27   ` Krzysztof Kozlowski
2023-07-22  9:27     ` Krzysztof Kozlowski
2023-07-22 21:52     ` Duje Mihanović
2023-07-24 14:14     ` Rob Herring
2023-07-24 21:37       ` Duje Mihanović
2023-07-21 20:37 ` [PATCH 10/10] MAINTAINERS: add myself as Marvell PXA1908 maintainer Duje Mihanović

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZL4+2bS3cVGq7q/5@smile.fi.intel.com \
    --to=andriy.shevchenko@intel.com \
    --cc=afaerber@suse.com \
    --cc=duje.mihanovic@skole.hr \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=phone-devel@vger.kernel.org \
    --cc=sboyd@kernel.org \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.