Re: [PATCH v2 07/16] clk: st: Remove uninitialized_var() usage
Quoting Kees Cook (2020-06-19 20:29:58)
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings (e.g.
> "unused variable"). If the compiler thinks it is uninitialized, either
> simply initialize the variable or make compiler changes. As a precursor
> to removing[2] this[3] macro[4], just remove this variable since it was
> actually unused:
>
> drivers/clk/st/clkgen-fsyn.c: In function \u2018quadfs_set_rate\u2019:
> drivers/clk/st/clkgen-fsyn.c:793:6: warning: unused variable \u2018i\u2019
> [-Wunused-variable]
> 793 | int i;
> | ^
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-gli...@google.com/
> [2]
> https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1tgqcr5vqkczwj0qxk6cernou6eedsuda...@mail.gmail.com/
> [3]
> https://lore.kernel.org/lkml/ca+55afwgbgqhbp1fkxvrkepzyr5j8n1vkt1vzdz9knmpuxh...@mail.gmail.com/
> [4]
> https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yvju65tplgn_ybynv0ve...@mail.gmail.com/
>
> Fixes: 5f7aa9071e93 ("clk: st: Support for QUADFS inside ClockGenB/C/D/E/F")
> Signed-off-by: Kees Cook
> ---
Acked-by: Stephen Boyd
Re: [PATCH v2 09/16] clk: spear: Remove uninitialized_var() usage
Quoting Kees Cook (2020-06-19 20:30:00)
> Using uninitialized_var() is dangerous as it papers over real bugs[1]
> (or can in the future), and suppresses unrelated compiler warnings (e.g.
> "unused variable"). If the compiler thinks it is uninitialized, either
> simply initialize the variable or make compiler changes. As a precursor
> to removing[2] this[3] macro[4], initialize "i" to zero. The compiler
> warning was not a false positive, since clk_pll_set_rate()'s call to
> clk_pll_round_rate_index() will always fail (since "prate" is NULL), so
> "i" was never being initialized.
>
> [1] https://lore.kernel.org/lkml/20200603174714.192027-1-gli...@google.com/
> [2]
> https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1tgqcr5vqkczwj0qxk6cernou6eedsuda...@mail.gmail.com/
> [3]
> https://lore.kernel.org/lkml/ca+55afwgbgqhbp1fkxvrkepzyr5j8n1vkt1vzdz9knmpuxh...@mail.gmail.com/
> [4]
> https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yvju65tplgn_ybynv0ve...@mail.gmail.com/
>
> Fixes: 7d4998f71b29 ("clk: SPEAr: Vco-pll: Fix compilation warning")
> Signed-off-by: Kees Cook
> ---
Acked-by: Stephen Boyd
Re: [PATCH 06/10] clk: st: Remove uninitialized_var() usage
Quoting Kees Cook (2020-06-03 16:31:59) > Using uninitialized_var() is dangerous as it papers over real bugs[1] > (or can in the future), and suppresses unrelated compiler warnings (e.g. > "unused variable"). If the compiler thinks it is uninitialized, either > simply initialize the variable or make compiler changes. As a precursor > to removing[2] this[3] macro[4], just remove this variable since it was > actually unused: > > drivers/clk/st/clkgen-fsyn.c: In function \u2018quadfs_set_rate\u2019: > drivers/clk/st/clkgen-fsyn.c:793:6: warning: unused variable \u2018i\u2019 > [-Wunused-variable] > 793 | int i; > | ^ > > [1] https://lore.kernel.org/lkml/20200603174714.192027-1-gli...@google.com/ > [2] > https://lore.kernel.org/lkml/CA+55aFw+Vbj0i=1tgqcr5vqkczwj0qxk6cernou6eedsuda...@mail.gmail.com/ > [3] > https://lore.kernel.org/lkml/ca+55afwgbgqhbp1fkxvrkepzyr5j8n1vkt1vzdz9knmpuxh...@mail.gmail.com/ > [4] > https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yvju65tplgn_ybynv0ve...@mail.gmail.com/ > > Signed-off-by: Kees Cook > --- Acked-by: Stephen Boyd
Re: [PATCH 2/4] r8169: Get and enable optional ether_clk clock
Quoting Hans de Goede (2018-08-29 10:09:57) > Hi, > > On 27-08-18 21:14, Stephen Boyd wrote: > > Quoting Hans de Goede (2018-08-27 11:53:19) > >> On 27-08-18 20:47, Stephen Boyd wrote: > >>> How would you know that a clk device driver hasn't probed yet and isn't > >>> the driver that's actually providing the clk to this device on x86 > >>> systems? With DT systems we can figure that out by looking at the DT and > >>> seeing if the device driver requesting the clk has the clocks property. > >>> On x86 systems it's all clkdev which doesn't really lend itself to > >>> solving this problem. > >> > >> Right on x86 the assumption is that the clk driver will be builtin and > >> will probe before the consumer. In this case that is true as the > >> pmc-atom-clk driver can only be builtin and its platform device is > >> instantiated from the acpi_lpss code and acpi init happens before > >> the PCI bus is scanned. > > > > If we can go with this assumption then we can make the optional clk API > > work even on clkdev based systems. Maybe if x86 had some way of > > indicating that all builtin clks are registered? > > Unfortunately there is no such thing I'm afraid. Ugh! > > > That might work but > > it's not very clean. Or if we could check to see if we're running on an > > ACPI based system in clkdev we could use that to assume that clk_get() > > will only be called after all providers have registered their lookups. > > Yes some check for x86 + ACPI (ARM also uses ACPI, but there we > should no do this AFAICT) is probably best. That or not use the > new optional clk API on x86, but that means that any cross platform > driver cannot use it, which would be a pain. Right. The optional clk API will be not so great until we can get ACPI to move way from clkdev. > > BTW does your Acked-by indicate you are ok with merging this series > through the netdev tree as I suggested in the cover-letter? If so > can I also add your Acked-by to the 3th patch ? > Yep, I thought I did that but now I've really done it.
Re: [PATCH 3/4] clk: x86: Stop marking clocks as CLK_IS_CRITICAL
Quoting Hans de Goede (2018-08-27 07:31:59)
> Commit d31fd43c0f9a ("clk: x86: Do not gate clocks enabled by the
> firmware"), which added the code to mark clocks as CLK_IS_CRITICAL, causes
> all unclaimed PMC clocks on Cherry Trail devices to be on all the time,
> resulting on the device not being able to reach S0i3 when suspended.
>
> The reason for this commit is that on some Bay Trail / Cherry Trail devices
> the r8169 ethernet controller uses pmc_plt_clk_4. Now that the clk-pmc-atom
> driver exports an "ether_clk" alias for pmc_plt_clk_4 and the r8169 driver
> has been modified to get and enable this clock (if present) the marking of
> the clocks as CLK_IS_CRITICAL is no longer necessary.
>
> This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail
> devices not being able to reach S0i3 greatly decreasing their battery
> drain when suspended.
>
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=193891#c102
> Cc: Johannes Stezenbach
> Cc: Carlo Caione
> Signed-off-by: Hans de Goede
> ---
Acked-by: Stephen Boyd
Re: [PATCH 2/4] r8169: Get and enable optional ether_clk clock
Quoting Hans de Goede (2018-08-27 11:53:19) > On 27-08-18 20:47, Stephen Boyd wrote: > > How would you know that a clk device driver hasn't probed yet and isn't > > the driver that's actually providing the clk to this device on x86 > > systems? With DT systems we can figure that out by looking at the DT and > > seeing if the device driver requesting the clk has the clocks property. > > On x86 systems it's all clkdev which doesn't really lend itself to > > solving this problem. > > Right on x86 the assumption is that the clk driver will be builtin and > will probe before the consumer. In this case that is true as the > pmc-atom-clk driver can only be builtin and its platform device is > instantiated from the acpi_lpss code and acpi init happens before > the PCI bus is scanned. If we can go with this assumption then we can make the optional clk API work even on clkdev based systems. Maybe if x86 had some way of indicating that all builtin clks are registered? That might work but it's not very clean. Or if we could check to see if we're running on an ACPI based system in clkdev we could use that to assume that clk_get() will only be called after all providers have registered their lookups. > > We have the same problem with ACPI OpRegions and drivers who have > _PS0 / _PS3 methods using those OpRegions and the "solution" so far > is the same, make sure all drivers providing OpRegion handlers are > builtin. > > Basically we (x86) miss the nice dependency info and complete hw > description devicetree gives, so we rely on initialization order > for some of this. I added the -EPROBE_DEFER handling for completeness > sake / for other platforms, as you point out it will never trigger > on x86. > Thanks for the info!
Re: [PATCH 2/4] r8169: Get and enable optional ether_clk clock
Quoting Hans de Goede (2018-08-27 07:31:58)
> On some boards a platform clock is used as clock for the r8169 chip,
> this commit adds support for getting and enabling this clock (assuming
> it has an "ether_clk" alias set on it).
>
> This is related to commit d31fd43c0f9a ("clk: x86: Do not gate clocks
> enabled by the firmware") which is a previous attempt to fix this for some
> x86 boards, but this causes all Cherry Trail SoC using boards to not reach
> there lowest power states when suspending.
>
> This commit (together with an atom-pmc-clk driver commit adding the alias)
> fixes things properly by making the r8169 get the clock and enable it when
> it needs it.
>
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=193891#c102
> Cc: Johannes Stezenbach
> Cc: Carlo Caione
> Signed-off-by: Hans de Goede
Acked-by: Stephen Boyd
> @@ -7614,6 +7616,11 @@ static void rtl_hw_initialize(struct rtl8169_private
> *tp)
> }
> }
>
> +static void rtl_disable_clk(void *data)
> +{
> + clk_disable_unprepare(data);
> +}
> +
> static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id
> *ent)
> {
> const struct rtl_cfg_info *cfg = rtl_cfg_infos + ent->driver_data;
> @@ -7647,6 +7654,32 @@ static int rtl_init_one(struct pci_dev *pdev, const
> struct pci_device_id *ent)
> mii->reg_num_mask = 0x1f;
> mii->supports_gmii = cfg->has_gmii;
>
> + /* Get the *optional* external "ether_clk" used on some boards */
> + tp->clk = devm_clk_get(&pdev->dev, "ether_clk");
An "optional" clk API is in flight, but it's easier to wait for that to
merge and then this to be updated, so just take that as an FYI. It would
be interesting to see how to support optional clks with clkdev lookups
though, which would be needed in this case.
How would you know that a clk device driver hasn't probed yet and isn't
the driver that's actually providing the clk to this device on x86
systems? With DT systems we can figure that out by looking at the DT and
seeing if the device driver requesting the clk has the clocks property.
On x86 systems it's all clkdev which doesn't really lend itself to
solving this problem.
> + if (IS_ERR(tp->clk)) {
> + rc = PTR_ERR(tp->clk);
Re: [PATCH 1/4] clk: x86: add "ether_clk" alias for Bay Trail / Cherry Trail
Quoting Hans de Goede (2018-08-27 07:31:57)
> Commit d31fd43c0f9a ("clk: x86: Do not gate clocks enabled by the
> firmware") causes all unclaimed PMC clocks on Cherry Trail devices to be on
> all the time, resulting on the device not being able to reach S0i2 or S0i3
> when suspended.
>
> The reason for this commit is that on some Bay Trail / Cherry Trail devices
> the ethernet controller uses pmc_plt_clk_4. This commit adds an "ether_clk"
> alias, so that the relevant ethernet drivers can try to (optionally) use
> this, without needing X86 specific code / hacks, thus fixing ethernet on
> these devices without breaking S0i3 support.
>
> This commit uses clkdev_hw_create() to create the alias, mirroring the code
> for the already existing "mclk" alias for pmc_plt_clk_3.
>
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=193891#c102
> Cc: Johannes Stezenbach
> Cc: Carlo Caione
> Signed-off-by: Hans de Goede
> ---
Acked-by: Stephen Boyd
[PATCH] net: ks8851: Drop eeprom_size structure member
After commit 51b7b1c34e19 (KSZ8851-SNL: Add ethtool support for
EEPROM via eeprom_93cx6, 2011-11-21) this structure member is
unused. Delete it.
Signed-off-by: Stephen Boyd
---
Found while debugging an eeprom issue.
drivers/net/ethernet/micrel/ks8851.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/net/ethernet/micrel/ks8851.c
b/drivers/net/ethernet/micrel/ks8851.c
index e7e1aff40bd9..955d69a8e8d3 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -84,7 +84,6 @@ union ks8851_tx_hdr {
* @rc_ier: Cached copy of KS_IER.
* @rc_ccr: Cached copy of KS_CCR.
* @rc_rxqcr: Cached copy of KS_RXQCR.
- * @eeprom_size: Companion eeprom size in Bytes, 0 if no eeprom
* @eeprom: 93CX6 EEPROM state for accessing on-board EEPROM.
* @vdd_reg: Optional regulator supplying the chip
* @vdd_io: Optional digital power supply for IO
@@ -120,7 +119,6 @@ struct ks8851_net {
u16 rc_ier;
u16 rc_rxqcr;
u16 rc_ccr;
- u16 eeprom_size;
struct mii_if_info mii;
struct ks8851_rxctrlrxctrl;
@@ -1533,11 +1531,6 @@ static int ks8851_probe(struct spi_device *spi)
/* cache the contents of the CCR register for EEPROM, etc. */
ks->rc_ccr = ks8851_rdreg16(ks, KS_CCR);
- if (ks->rc_ccr & CCR_EEPROM)
- ks->eeprom_size = 128;
- else
- ks->eeprom_size = 0;
-
ks8851_read_selftest(ks);
ks8851_init_mac(ks);
--
2.10.0.297.gf6727b0
[PATCH] net: qrtr: Mark 'buf' as little endian
Failure to mark this pointer as __le32 causes checkers like sparse to complain: net/qrtr/qrtr.c:274:16: warning: incorrect type in assignment (different base types) net/qrtr/qrtr.c:274:16:expected unsigned int [unsigned] [usertype] net/qrtr/qrtr.c:274:16:got restricted __le32 [usertype] net/qrtr/qrtr.c:275:16: warning: incorrect type in assignment (different base types) net/qrtr/qrtr.c:275:16:expected unsigned int [unsigned] [usertype] net/qrtr/qrtr.c:275:16:got restricted __le32 [usertype] net/qrtr/qrtr.c:276:16: warning: incorrect type in assignment (different base types) net/qrtr/qrtr.c:276:16:expected unsigned int [unsigned] [usertype] net/qrtr/qrtr.c:276:16:got restricted __le32 [usertype] Silence it. Cc: Bjorn Andersson Signed-off-by: Stephen Boyd --- net/qrtr/qrtr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c index c985ecbe9bd6..ae5ac175b2be 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -252,7 +252,7 @@ static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node, const int pkt_len = 20; struct qrtr_hdr *hdr; struct sk_buff *skb; - u32 *buf; + __le32 *buf; skb = alloc_skb(QRTR_HDR_SIZE + pkt_len, GFP_KERNEL); if (!skb) @@ -269,7 +269,7 @@ static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node, hdr->dst_node_id = cpu_to_le32(dst_node); hdr->dst_port_id = cpu_to_le32(QRTR_PORT_CTRL); - buf = (u32 *)skb_put(skb, pkt_len); + buf = (__le32 *)skb_put(skb, pkt_len); memset(buf, 0, pkt_len); buf[0] = cpu_to_le32(QRTR_TYPE_RESUME_TX); buf[1] = cpu_to_le32(src_node); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 2/6] net: ethernet: ti: cpts: add support for ext rftclk selection
On 12/06, Grygorii Strashko wrote:
> Subject: [PATCH] cpts refclk sel
>
> Signed-off-by: Grygorii Strashko
> ---
> arch/arm/boot/dts/keystone-k2e-netcp.dtsi | 10 +-
> drivers/net/ethernet/ti/cpts.c| 52
> ++-
> 2 files changed, 60 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
> b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
> index 919e655..b27aa22 100644
> --- a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
> +++ b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
> @@ -138,7 +138,7 @@ netcp: netcp@2400 {
> /* NetCP address range */
> ranges = <0 0x2400 0x100>;
>
> - clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
> + clocks = <&clkpa>, <&clkcpgmac>, <&cpts_mux>;
> clock-names = "pa_clk", "ethss_clk", "cpts";
> dma-coherent;
>
> @@ -162,6 +162,14 @@ netcp: netcp@2400 {
> cpts-ext-ts-inputs = <6>;
> cpts-ts-comp-length;
>
> + cpts_mux: cpts_refclk_mux {
> + #clock-cells = <0>;
> + clocks = <&chipclk12>, <&chipclk13>;
> + cpts-mux-tbl = <0>, <1>;
> + assigned-clocks = <&cpts_mux>;
> + assigned-clock-parents = <&chipclk12>;
Is there a binding update? Why the subnode? Why not have it as
part of the netcp node? Does the cpts-mux-tbl property change?
> + };
> +
> interfaces {
> gbe0: interface-0 {
> slave-port = <0>;
> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
> index 938de22..ef94316 100644
> --- a/drivers/net/ethernet/ti/cpts.c
> +++ b/drivers/net/ethernet/ti/cpts.c
> @@ -17,6 +17,7 @@
> * along with this program; if not, write to the Free Software
> * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> */
> +#include
> #include
> #include
> #include
> @@ -672,6 +673,7 @@ int cpts_register(struct cpts *cpts)
> cpts->phc_index = ptp_clock_index(cpts->clock);
>
> schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period);
> +
Maybe in another patch.
> return 0;
>
> err_ptp:
> @@ -741,6 +743,54 @@ static void cpts_calc_mult_shift(struct cpts *cpts)
>freq, cpts->cc_mult, cpts->cc.shift, (ns - NSEC_PER_SEC));
> }
>
> +static int cpts_of_mux_clk_setup(struct cpts *cpts, struct device_node *node)
> +{
> + unsigned int num_parents;
> + const char **parent_names;
> + struct device_node *refclk_np;
> + void __iomem *reg;
> + struct clk *clk;
> + u32 *mux_table;
> + int ret;
> +
> + refclk_np = of_get_child_by_name(node, "cpts_refclk_mux");
> + if (!refclk_np)
> + return -EINVAL;
> +
> + num_parents = of_clk_get_parent_count(refclk_np);
> + if (num_parents < 1) {
> + dev_err(cpts->dev, "mux-clock %s must have parents\n",
> + refclk_np->name);
> + return -EINVAL;
> + }
> + parent_names = devm_kzalloc(cpts->dev, (sizeof(char *) * num_parents),
> + GFP_KERNEL);
> + if (!parent_names)
> + return -ENOMEM;
> +
> + of_clk_parent_fill(refclk_np, parent_names, num_parents);
> +
> + mux_table = devm_kzalloc(cpts->dev, sizeof(*mux_table) * (32 + 1),
> + GFP_KERNEL);
> + if (!mux_table)
> + return -ENOMEM;
> +
> + ret = of_property_read_variable_u32_array(refclk_np, "cpts-mux-tbl",
> + mux_table, 1, 32);
> + if (ret < 0)
> + return ret;
> +
> + reg = &cpts->reg->rftclk_sel;
> +
> + clk = clk_register_mux_table(cpts->dev, refclk_np->name,
> + parent_names, num_parents,
> + 0, reg, 0, 0x1F, 0, mux_table, NULL);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + return of_clk_add_provider(refclk_np, of_clk_src_simple_get, clk);
Can you please use the clk_hw APIs instead?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v5 2/6] clk: gxbb: expose MPLL2 clock for use by DT
On 09/06, Martin Blumenstingl wrote: > This exposes the MPLL2 clock as this is one of the input clocks of the > ethernet controller's internal mux. > > Signed-off-by: Martin Blumenstingl > --- Acked-by: Stephen Boyd -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v3 4/5] net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC
On 08/28, Martin Blumenstingl wrote:
> +static int meson8b_init_clk(struct meson8b_dwmac *dwmac)
> +{
> + struct clk_init_data init;
> + int i, ret;
> + struct device *dev = &dwmac->pdev->dev;
> + char clk_name[32];
> + const char *clk_div_parents[1];
> + const char *mux_parent_names[MUX_CLK_NUM_PARENTS];
> + static struct clk_div_table clk_25m_div_table[] = {
> + { .val = 0, .div = 5 },
> + { .val = 1, .div = 10 },
> + { /* sentinel */ },
> + };
> +
> + /* get the mux parents from DT */
> + for (i = 0; i < MUX_CLK_NUM_PARENTS; i++) {
> + char name[16];
> +
> + snprintf(name, sizeof(name), "clkin%d", i);
> + dwmac->m250_mux_parent[i] = devm_clk_get(dev, name);
> + if (IS_ERR(dwmac->m250_mux_parent[i])) {
> + ret = PTR_ERR(dwmac->m250_mux_parent[i]);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "Missing clock %s\n", name);
> + return ret;
> + }
> +
> + mux_parent_names[i] =
> + __clk_get_name(dwmac->m250_mux_parent[i]);
> + }
> +
> + /* create the m250_mux */
> + snprintf(clk_name, sizeof(clk_name), "%s#m250_sel", dev_name(dev));
> + init.name = clk_name;
> + init.ops = &clk_mux_ops;
> + init.flags = CLK_IS_BASIC;
Please don't use this flag unless you need it.
> + init.parent_names = mux_parent_names;
> + init.num_parents = MUX_CLK_NUM_PARENTS;
> +
> + dwmac->m250_mux.reg = dwmac->regs + PRG_ETH0;
> + dwmac->m250_mux.shift = PRG_ETH0_CLK_M250_SEL_SHIFT;
> + dwmac->m250_mux.mask = PRG_ETH0_CLK_M250_SEL_MASK;
> + dwmac->m250_mux.flags = 0;
> + dwmac->m250_mux.table = NULL;
> + dwmac->m250_mux.hw.init = &init;
> +
> + dwmac->m250_mux_clk = devm_clk_register(dev, &dwmac->m250_mux.hw);
> + if (WARN_ON(PTR_ERR_OR_ZERO(dwmac->m250_mux_clk)))
Why not if(WARN_ON(IS_ERR())? The OR_ZERO part seems confusing.
> + return PTR_ERR(dwmac->m250_mux_clk);
> +
> + /* create the m250_div */
> + snprintf(clk_name, sizeof(clk_name), "%s#m250_div", dev_name(dev));
> + init.name = devm_kstrdup(dev, clk_name, GFP_KERNEL);
> + init.ops = &clk_divider_ops;
> + init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
> + clk_div_parents[0] = __clk_get_name(dwmac->m250_mux_clk);
> + init.parent_names = clk_div_parents;
> + init.num_parents = ARRAY_SIZE(clk_div_parents);
> +
> + dwmac->m250_div.reg = dwmac->regs + PRG_ETH0;
> + dwmac->m250_div.shift = PRG_ETH0_CLK_M250_DIV_SHIFT;
> + dwmac->m250_div.width = PRG_ETH0_CLK_M250_DIV_WIDTH;
> + dwmac->m250_div.hw.init = &init;
> + dwmac->m250_div.flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO;
> +
> + dwmac->m250_div_clk = devm_clk_register(dev, &dwmac->m250_div.hw);
We've been trying to move away from devm_clk_register() to
devm_clk_hw_register() so that clk providers aren't also clk
consumers. Obviously in this case this driver is a provider and a
consumer, so this isn't as important. Kevin did something similar
in the mmc driver, so I'll reiterate what I said on that patch.
Perhaps we should make __clk_create_clk() into a real clk
provider API so that we can use devm_clk_hw_register() here and
then generate a clk for this device. That would allow us to have
proper consumer tracking without relying on the clk that is
returned from clk_register() (the intent is to make that clk
instance internal to the framework).
> + if (WARN_ON(PTR_ERR_OR_ZERO(dwmac->m250_div_clk)))
> + return PTR_ERR(dwmac->m250_div_clk);
> +
> + /* create the m25_div */
> + snprintf(clk_name, sizeof(clk_name), "%s#m25_div", dev_name(dev));
> + init.name = devm_kstrdup(dev, clk_name, GFP_KERNEL);
> + init.ops = &clk_divider_ops;
> + init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
> + clk_div_parents[0] = __clk_get_name(dwmac->m250_div_clk);
> + init.parent_names = clk_div_parents;
> + init.num_parents = ARRAY_SIZE(clk_div_parents);
> +
> + dwmac->m25_div.reg = dwmac->regs + PRG_ETH0;
> + dwmac->m25_div.shift = PRG_ETH0_CLK_M25_DIV_SHIFT;
> + dwmac->m25_div.width = PRG_ETH0_CLK_M25_DIV_WIDTH;
> + dwmac->m25_div.table = clk_25m_div_table;
> + dwmac->m25_div.hw.init = &init;
> + dwmac->m25_div.flags = CLK_DIVIDER_ALLOW_ZERO;
> +
> + dwmac->m25_div_clk = devm_clk_register(dev, &dwmac->m25_div.hw);
> + if (WARN_ON(PTR_ERR_OR_ZERO(dwmac->m25_div_clk)))
> + return PTR_ERR(dwmac->m25_div_clk);
> +
> + return 0;
This could be return WARN_ON(PTR_ERR_OR_ZERO(...))
> +
> +static int meson8b_dwmac_probe(struct platform_device *pdev)
> +{
> + struct plat_stmmacenet_data *plat_dat;
> + struct stmmac_resources stmmac_res;
> + struct resource *res;
> + struct meson8b_dwmac *dwmac;
> + int ret;
> +
> + ret = stmmac_get_platform_r
Re: [v7, 4/5] powerpc/fsl: move mpc85xx.h to include/linux/fsl
On 03/31/2016 08:07 PM, Yangbo Lu wrote: > drivers/clk/clk-qoriq.c | 3 +-- > For clk part: Acked-by: Stephen Boyd -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 8/9] clk: rockchip: associate SCLK_MAC_PLL and disable reparenting on rk3036
On 03/13, Caesar Wang wrote: > From: Heiko Stuebner > > The emac needs constant and very specific rate but the possible PLL-sources > are very limited, so we expect the PLL source to be set manually on per > board and don't want it to get changed in an automatic way later. > So add the necessary clock-id and disable reparenting on set_rate calls. > > Signed-off-by: Heiko Stuebner > Signed-off-by: Caesar Wang > --- Acked-by: Stephen Boyd -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 6/9] clk: rockchip: associate the rk3036 HCLK_EMAC clock-id
On 03/13, Caesar Wang wrote: > From: Xing Zheng > > Associate the new clock id the clock. > > Signed-off-by: Xing Zheng > Signed-off-by: Caesar Wang > --- Acked-by: Stephen Boyd -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH] stmmac: ipq806x: Return error values instead of pointers
Typically we return error pointers when we want to use those
pointers in the non-error case, but this function is just
returning error pointers or NULL for success. Change the style to
plain int to follow normal kernel coding styles.
Cc: Joachim Eastwood
Signed-off-by: Stephen Boyd
---
.../net/ethernet/stmicro/stmmac/dwmac-ipq806x.c| 24 ++
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c
b/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c
index 82de68b1a452..36d3355f2fb0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-ipq806x.c
@@ -198,19 +198,19 @@ static int ipq806x_gmac_set_speed(struct ipq806x_gmac
*gmac, unsigned int speed)
return 0;
}
-static void *ipq806x_gmac_of_parse(struct ipq806x_gmac *gmac)
+static int ipq806x_gmac_of_parse(struct ipq806x_gmac *gmac)
{
struct device *dev = &gmac->pdev->dev;
gmac->phy_mode = of_get_phy_mode(dev->of_node);
if (gmac->phy_mode < 0) {
dev_err(dev, "missing phy mode property\n");
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
}
if (of_property_read_u32(dev->of_node, "qcom,id", &gmac->id) < 0) {
dev_err(dev, "missing qcom id property\n");
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
}
/* The GMACs are called 1 to 4 in the documentation, but to simplify the
@@ -219,13 +219,13 @@ static void *ipq806x_gmac_of_parse(struct ipq806x_gmac
*gmac)
*/
if (gmac->id < 0 || gmac->id > 3) {
dev_err(dev, "invalid gmac id\n");
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
}
gmac->core_clk = devm_clk_get(dev, "stmmaceth");
if (IS_ERR(gmac->core_clk)) {
dev_err(dev, "missing stmmaceth clk property\n");
- return gmac->core_clk;
+ return PTR_ERR(gmac->core_clk);
}
clk_set_rate(gmac->core_clk, 26600);
@@ -234,18 +234,16 @@ static void *ipq806x_gmac_of_parse(struct ipq806x_gmac
*gmac)
"qcom,nss-common");
if (IS_ERR(gmac->nss_common)) {
dev_err(dev, "missing nss-common node\n");
- return gmac->nss_common;
+ return PTR_ERR(gmac->nss_common);
}
/* Setup the register map for the qsgmii csr registers */
gmac->qsgmii_csr = syscon_regmap_lookup_by_phandle(dev->of_node,
"qcom,qsgmii-csr");
- if (IS_ERR(gmac->qsgmii_csr)) {
+ if (IS_ERR(gmac->qsgmii_csr))
dev_err(dev, "missing qsgmii-csr node\n");
- return gmac->qsgmii_csr;
- }
- return NULL;
+ return PTR_ERR_OR_ZERO(gmac->qsgmii_csr);
}
static void ipq806x_gmac_fix_mac_speed(void *priv, unsigned int speed)
@@ -262,7 +260,7 @@ static int ipq806x_gmac_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct ipq806x_gmac *gmac;
int val;
- void *err;
+ int err;
val = stmmac_get_platform_resources(pdev, &stmmac_res);
if (val)
@@ -279,9 +277,9 @@ static int ipq806x_gmac_probe(struct platform_device *pdev)
gmac->pdev = pdev;
err = ipq806x_gmac_of_parse(gmac);
- if (IS_ERR(err)) {
+ if (err) {
dev_err(dev, "device tree parsing error\n");
- return PTR_ERR(err);
+ return err;
}
regmap_write(gmac->qsgmii_csr, QSGMII_PCS_CAL_LCKDT_CTL,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
