Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] scsi: ufs: skip shutdown if hba is not powered
From: Stanley Chu @ 2019-09-18  4:20 UTC (permalink / raw)
  To: linux-scsi, martin.petersen, avri.altman, alim.akhtar,
	pedrom.sousa, jejb
  Cc: andy.teng, chun-hung.wu, kuohong.wang, linux-mediatek, peter.wang,
	matthias.bgg, Stanley Chu, linux-arm-kernel, beanhuo

In some cases, hba may go through shutdown flow without successful
initialization and then make system hang.

For example, if ufshcd_change_power_mode() gets error and
leads to ufshcd_hba_exit() to release resources of the host,
future shutdown flow may hang the system since the host register
will be accessed in unpowered state.

To solve this issue, simply add checking to skip shutdown for above
kind of situation.

Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
 drivers/scsi/ufs/ufshcd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 60a24fe908e7..a601ce0f6195 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -8121,6 +8121,9 @@ int ufshcd_shutdown(struct ufs_hba *hba)
 {
 	int ret = 0;
 
+	if (!hba->is_powered)
+		goto out;
+
 	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
 		goto out;
 
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v5 7/8] clk: Add common clock driver for BM1880 SoC
From: Stephen Boyd @ 2019-09-18  4:47 UTC (permalink / raw)
  To: Manivannan Sadhasivam, mturquette, robh+dt
  Cc: devicetree, Manivannan Sadhasivam, darren.tsao, linux-kernel,
	linux-arm-kernel, fisher.cheng, alec.lin, linux-clk, haitao.suo
In-Reply-To: <20190916161447.32715-8-manivannan.sadhasivam@linaro.org>

Quoting Manivannan Sadhasivam (2019-09-16 09:14:46)
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 801fa1cd0321..e70c64e43ff9 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -139,6 +139,13 @@ config COMMON_CLK_SI570
>           This driver supports Silicon Labs 570/571/598/599 programmable
>           clock generators.
>  
> +config COMMON_CLK_BM1880
> +       bool "Clock driver for Bitmain BM1880 SoC"

Can it be tristate?

> +       depends on ARCH_BITMAIN || COMPILE_TEST
> +       default ARCH_BITMAIN
> +       help
> +         This driver supports the clocks on Bitmain BM1880 SoC.
> +
>  config COMMON_CLK_CDCE706
>         tristate "Clock driver for TI CDCE706 clock synthesizer"
>         depends on I2C
> diff --git a/drivers/clk/clk-bm1880.c b/drivers/clk/clk-bm1880.c
> new file mode 100644
> index 000000000000..3b10de929fd4
> --- /dev/null
> +++ b/drivers/clk/clk-bm1880.c
> @@ -0,0 +1,966 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Bitmain BM1880 SoC clock driver
> + *
> + * Copyright (c) 2019 Linaro Ltd.
> + * Author: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#include <dt-bindings/clock/bm1880-clock.h>
> +
> +#define BM1880_CLK_MPLL_CTL    0x00
> +#define BM1880_CLK_SPLL_CTL    0x04
> +#define BM1880_CLK_FPLL_CTL    0x08
> +#define BM1880_CLK_DDRPLL_CTL  0x0c
> +
> +#define BM1880_CLK_ENABLE0     0x00
> +#define BM1880_CLK_ENABLE1     0x04
> +#define BM1880_CLK_SELECT      0x20
> +#define BM1880_CLK_DIV0                0x40
> +#define BM1880_CLK_DIV1                0x44
> +#define BM1880_CLK_DIV2                0x48
> +#define BM1880_CLK_DIV3                0x4c
> +#define BM1880_CLK_DIV4                0x50
> +#define BM1880_CLK_DIV5                0x54
> +#define BM1880_CLK_DIV6                0x58
> +#define BM1880_CLK_DIV7                0x5c
> +#define BM1880_CLK_DIV8                0x60
> +#define BM1880_CLK_DIV9                0x64
> +#define BM1880_CLK_DIV10       0x68
> +#define BM1880_CLK_DIV11       0x6c
> +#define BM1880_CLK_DIV12       0x70
> +#define BM1880_CLK_DIV13       0x74
> +#define BM1880_CLK_DIV14       0x78
> +#define BM1880_CLK_DIV15       0x7c
> +#define BM1880_CLK_DIV16       0x80
> +#define BM1880_CLK_DIV17       0x84
> +#define BM1880_CLK_DIV18       0x88
> +#define BM1880_CLK_DIV19       0x8c
> +#define BM1880_CLK_DIV20       0x90
> +#define BM1880_CLK_DIV21       0x94
> +#define BM1880_CLK_DIV22       0x98
> +#define BM1880_CLK_DIV23       0x9c
> +#define BM1880_CLK_DIV24       0xa0
> +#define BM1880_CLK_DIV25       0xa4
> +#define BM1880_CLK_DIV26       0xa8
> +#define BM1880_CLK_DIV27       0xac
> +#define BM1880_CLK_DIV28       0xb0
> +
> +#define to_bm1880_pll_clk(_hw) container_of(_hw, struct bm1880_pll_hw_clock, hw)
> +#define to_bm1880_div_clk(_hw) container_of(_hw, struct bm1880_div_hw_clock, hw)
> +
> +static DEFINE_SPINLOCK(bm1880_clk_lock);
> +
> +struct bm1880_clock_data {
> +       void __iomem *pll_base;
> +       void __iomem *sys_base;
> +       struct clk_hw_onecell_data *clk_data;

Please call it hw_data or onecell_data instead so that the code doesn't
read as clk_data->clk_data. Also, probably can just make it part of the
same struct instead of a pointer inside so that it can all be allocated
in one big chunk instead of in two.

> +};
> +
> +struct bm1880_gate_clock {
> +       unsigned int    id;
> +       const char      *name;
> +       const char      *parent;
> +       u32             gate_reg;
> +       s8              gate_shift;
> +       unsigned long   flags;
> +};
> +
> +struct bm1880_mux_clock {
> +       unsigned int    id;
> +       const char      *name;
> +       const char      * const *parents;
> +       s8              num_parents;
> +       u32             reg;
> +       s8              shift;
> +       unsigned long   flags;
> +};
> +
> +struct bm1880_div_clock {
> +       unsigned int    id;
> +       const char      *name;
> +       u32             reg;
> +       u8              shift;
> +       u8              width;
> +       u32             initval;
> +       const struct clk_div_table *table;
> +       unsigned long flags;
> +};
> +
> +struct bm1880_div_hw_clock {
> +       struct bm1880_div_clock div;
> +       void __iomem *base;
> +       spinlock_t *lock;
> +       struct clk_hw hw;
> +       struct clk_init_data init;
> +};
> +
> +struct bm1880_composite_clock {
> +       unsigned int    id;
> +       const char      *name;
> +       const char      *parent;
> +       const char      * const *parents;
> +       unsigned int    num_parents;
> +       unsigned long   flags;
> +
> +       u32             gate_reg;
> +       u32             mux_reg;
> +       u32             div_reg;
> +
> +       s8              gate_shift;
> +       s8              mux_shift;
> +       s8              div_shift;
> +       s8              div_width;
> +       s16             div_initval;
> +       const struct clk_div_table *table;
> +};
> +
> +struct bm1880_pll_clock {
> +       unsigned int    id;
> +       const char      *name;
> +       u32             reg;
> +       unsigned long   flags;
> +};
> +
> +struct bm1880_pll_hw_clock {
> +       struct bm1880_pll_clock pll;
> +       void __iomem *base;
> +       struct clk_hw hw;
> +       struct clk_init_data init;
> +};
> +
> +static const struct clk_ops bm1880_pll_ops;
> +static const struct clk_ops bm1880_clk_div_ops;
> +
> +#define GATE_DIV(_id, _name, _parent, _gate_reg, _gate_shift, _div_reg,        \
> +                       _div_shift, _div_width, _div_initval, _table,   \
> +                       _flags) {                                       \
> +               .id = _id,                                              \
> +               .parent = _parent,                                      \
> +               .name = _name,                                          \
> +               .gate_reg = _gate_reg,                                  \
> +               .gate_shift = _gate_shift,                              \
> +               .div_reg = _div_reg,                                    \
> +               .div_shift = _div_shift,                                \
> +               .div_width = _div_width,                                \
> +               .div_initval = _div_initval,                            \
> +               .table = _table,                                        \
> +               .mux_shift = -1,                                        \
> +               .flags = _flags,                                        \
> +       }
> +
> +#define GATE_MUX(_id, _name, _parents, _gate_reg, _gate_shift,         \
> +                       _mux_reg, _mux_shift, _flags) {                 \
> +               .id = _id,                                              \
> +               .parents = _parents,                                    \
> +               .num_parents = ARRAY_SIZE(_parents),                    \
> +               .name = _name,                                          \
> +               .gate_reg = _gate_reg,                                  \
> +               .gate_shift = _gate_shift,                              \
> +               .div_shift = -1,                                        \
> +               .mux_reg = _mux_reg,                                    \
> +               .mux_shift = _mux_shift,                                \
> +               .flags = _flags,                                        \
> +       }
> +
> +#define CLK_PLL(_id, _name, _parent, _reg, _flags) {                   \
> +               .pll.id = _id,                                          \
> +               .pll.name = _name,                                      \
> +               .pll.reg = _reg,                                        \
> +               .hw.init = CLK_HW_INIT_PARENTS_DATA(_name, _parent,     \
> +                                                   &bm1880_pll_ops,    \
> +                                                   _flags),            \
> +       }
> +
> +#define CLK_DIV(_id, _name, _parent, _reg, _shift, _width, _initval,   \
> +                               _table, _flags) {                       \
> +               .div.id = _id,                                          \
> +               .div.name = _name,                                      \
> +               .div.reg = _reg,                                        \
> +               .div.shift = _shift,                                    \
> +               .div.width = _width,                                    \
> +               .div.initval = _initval,                                \
> +               .div.table = _table,                                    \
> +               .hw.init = CLK_HW_INIT_HW(_name, _parent,               \
> +                                         &bm1880_clk_div_ops,          \
> +                                         _flags),                      \
> +       }
> +
> +static struct clk_parent_data bm1880_pll_parent[] = {
> +       { .fw_name = "osc", .name = "osc" },
> +};
> +
> +/*
> + * All PLL clocks are marked as CRITICAL, hence they are very crucial
> + * for the functioning of the SoC

Please add more information besides crucial to function of the clk.
Basically describe what the PLLs are clocking and why those child clks
aren't enabled or marked as critical themselves. The usage of
CLK_IS_CRITICAL is too liberal in this driver so this needs to be
cleaned up. For example, clk_mpll shouldn't be marked critical, just the
a53 CPU clk that can source from it should be marked critical because
it's for the CPU running code. It's also odd that we would register gate
clks or the a53 clks if we don't expect to ever turn those clks off. Can
that be avoided so that we don't need to mark anything critical for this
path?

> + */
> +static struct bm1880_pll_hw_clock bm1880_pll_clks[] = {
> +       CLK_PLL(BM1880_CLK_MPLL, "clk_mpll", bm1880_pll_parent,
> +               BM1880_CLK_MPLL_CTL, CLK_IS_CRITICAL),
> +       CLK_PLL(BM1880_CLK_SPLL, "clk_spll", bm1880_pll_parent,
> +               BM1880_CLK_SPLL_CTL, CLK_IS_CRITICAL),
> +       CLK_PLL(BM1880_CLK_FPLL, "clk_fpll", bm1880_pll_parent,
> +               BM1880_CLK_FPLL_CTL, CLK_IS_CRITICAL),
> +       CLK_PLL(BM1880_CLK_DDRPLL, "clk_ddrpll", bm1880_pll_parent,
> +               BM1880_CLK_DDRPLL_CTL, CLK_IS_CRITICAL),
> +};
> +
> +/*
> + * Clocks marked as CRITICAL are needed for the proper functioning
> + * of the SoC.
> + */
> +static const struct bm1880_gate_clock bm1880_gate_clks[] = {
> +       { BM1880_CLK_AHB_ROM, "clk_ahb_rom", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 2, CLK_IS_CRITICAL },
> +       { BM1880_CLK_AXI_SRAM, "clk_axi_sram", "clk_axi1",
> +         BM1880_CLK_ENABLE0, 3, CLK_IS_CRITICAL },
> +       { BM1880_CLK_DDR_AXI, "clk_ddr_axi", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 4, CLK_IS_CRITICAL },
> +       { BM1880_CLK_APB_EFUSE, "clk_apb_efuse", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 6, CLK_IS_CRITICAL },
> +       { BM1880_CLK_AXI5_EMMC, "clk_axi5_emmc", "clk_axi5",
> +         BM1880_CLK_ENABLE0, 7, 0 },
> +       { BM1880_CLK_AXI5_SD, "clk_axi5_sd", "clk_axi5",
> +         BM1880_CLK_ENABLE0, 10, 0 },
> +       { BM1880_CLK_AXI4_ETH0, "clk_axi4_eth0", "clk_axi4",
> +         BM1880_CLK_ENABLE0, 14, 0 },
> +       { BM1880_CLK_AXI4_ETH1, "clk_axi4_eth1", "clk_axi4",
> +         BM1880_CLK_ENABLE0, 16, 0 },
> +       { BM1880_CLK_AXI1_GDMA, "clk_axi1_gdma", "clk_axi1",
> +         BM1880_CLK_ENABLE0, 17, 0 },
> +       /* Don't gate GPIO clocks as it is not owned by the GPIO driver */
> +       { BM1880_CLK_APB_GPIO, "clk_apb_gpio", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 18, CLK_IGNORE_UNUSED },
> +       { BM1880_CLK_APB_GPIO_INTR, "clk_apb_gpio_intr", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 19, CLK_IGNORE_UNUSED },
> +       { BM1880_CLK_AXI1_MINER, "clk_axi1_miner", "clk_axi1",
> +         BM1880_CLK_ENABLE0, 21, 0 },
> +       { BM1880_CLK_AHB_SF, "clk_ahb_sf", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 22, 0 },
> +       { BM1880_CLK_SDMA_AXI, "clk_sdma_axi", "clk_axi5",
> +         BM1880_CLK_ENABLE0, 23, 0 },
> +       { BM1880_CLK_APB_I2C, "clk_apb_i2c", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 25, 0 },
> +       { BM1880_CLK_APB_WDT, "clk_apb_wdt", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE0, 26, 0 },
> +       { BM1880_CLK_APB_JPEG, "clk_apb_jpeg", "clk_axi6",
> +         BM1880_CLK_ENABLE0, 27, 0 },
> +       { BM1880_CLK_AXI5_NF, "clk_axi5_nf", "clk_axi5",
> +         BM1880_CLK_ENABLE0, 29, 0 },
> +       { BM1880_CLK_APB_NF, "clk_apb_nf", "clk_axi6",
> +         BM1880_CLK_ENABLE0, 30, 0 },
> +       { BM1880_CLK_APB_PWM, "clk_apb_pwm", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE1, 0, 0 },
> +       { BM1880_CLK_RV, "clk_rv", "clk_mux_rv",
> +         BM1880_CLK_ENABLE1, 1, 0 },
> +       { BM1880_CLK_APB_SPI, "clk_apb_spi", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE1, 2, 0 },
> +       { BM1880_CLK_UART_500M, "clk_uart_500m", "clk_div_uart_500m",
> +         BM1880_CLK_ENABLE1, 4, 0 },
> +       { BM1880_CLK_APB_UART, "clk_apb_uart", "clk_axi6",
> +         BM1880_CLK_ENABLE1, 5, 0 },
> +       { BM1880_CLK_APB_I2S, "clk_apb_i2s", "clk_axi6",
> +         BM1880_CLK_ENABLE1, 6, 0 },
> +       { BM1880_CLK_AXI4_USB, "clk_axi4_usb", "clk_axi4",
> +         BM1880_CLK_ENABLE1, 7, 0 },
> +       { BM1880_CLK_APB_USB, "clk_apb_usb", "clk_axi6",
> +         BM1880_CLK_ENABLE1, 8, 0 },
> +       { BM1880_CLK_12M_USB, "clk_12m_usb", "clk_div_12m_usb",
> +         BM1880_CLK_ENABLE1, 11, 0 },
> +       { BM1880_CLK_APB_VIDEO, "clk_apb_video", "clk_axi6",
> +         BM1880_CLK_ENABLE1, 12, 0 },
> +       { BM1880_CLK_APB_VPP, "clk_apb_vpp", "clk_axi6",
> +         BM1880_CLK_ENABLE1, 15, 0 },
> +       { BM1880_CLK_AXI6, "clk_axi6", "clk_mux_axi6",
> +         BM1880_CLK_ENABLE1, 21, CLK_IS_CRITICAL },
> +};
> +
> +static const char * const clk_a53_parents[] = { "clk_spll", "clk_mpll" };
> +static const char * const clk_rv_parents[] = { "clk_div_1_rv", "clk_div_0_rv" };
> +static const char * const clk_axi1_parents[] = { "clk_div_1_axi1", "clk_div_0_axi1" };
> +static const char * const clk_axi6_parents[] = { "clk_div_1_axi6", "clk_div_0_axi6" };

I sent some patches to make the basic clk types support the new way of
specifying parents. Can you use those patches instead of listing all
these strings?

> +
> +static const struct bm1880_mux_clock bm1880_mux_clks[] = {
> +       { BM1880_CLK_MUX_RV, "clk_mux_rv", clk_rv_parents, 2,
> +         BM1880_CLK_SELECT, 1, 0 },
> +       { BM1880_CLK_MUX_AXI6, "clk_mux_axi6", clk_axi6_parents, 2,
> +         BM1880_CLK_SELECT, 3, 0 },
> +};
> +
> +static const struct clk_div_table bm1880_div_table_0[] = {
> +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> +       { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> +       { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> +       { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> +       { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> +       { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> +       { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> +       { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> +       { 0, 0 }
> +};
> +
> +static const struct clk_div_table bm1880_div_table_1[] = {
> +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> +       { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> +       { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> +       { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> +       { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> +       { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> +       { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> +       { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> +       { 127, 128 }, { 0, 0 }
> +};
> +
> +static const struct clk_div_table bm1880_div_table_2[] = {
> +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> +       { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> +       { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> +       { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> +       { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> +       { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> +       { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> +       { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> +       { 127, 128 }, { 255, 256 }, { 0, 0 }
> +};
> +
> +static const struct clk_div_table bm1880_div_table_3[] = {
> +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> +       { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> +       { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> +       { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> +       { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> +       { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> +       { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> +       { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> +       { 127, 128 }, { 255, 256 }, { 511, 512 }, { 0, 0 }
> +};
> +
> +static const struct clk_div_table bm1880_div_table_4[] = {
> +       { 0, 1 }, { 1, 2 }, { 2, 3 }, { 3, 4 },
> +       { 4, 5 }, { 5, 6 }, { 6, 7 }, { 7, 8 },
> +       { 8, 9 }, { 9, 10 }, { 10, 11 }, { 11, 12 },
> +       { 12, 13 }, { 13, 14 }, { 14, 15 }, { 15, 16 },
> +       { 16, 17 }, { 17, 18 }, { 18, 19 }, { 19, 20 },
> +       { 20, 21 }, { 21, 22 }, { 22, 23 }, { 23, 24 },
> +       { 24, 25 }, { 25, 26 }, { 26, 27 }, { 27, 28 },
> +       { 28, 29 }, { 29, 30 }, { 30, 31 }, { 31, 32 },
> +       { 127, 128 }, { 255, 256 }, { 511, 512 }, { 65535, 65536 },
> +       { 0, 0 }
> +};
> +
> +/*
> + * Clocks marked as CRITICAL are needed for the proper functioning
> + * of the SoC.
> + */
> +static struct bm1880_div_hw_clock bm1880_div_clks[] = {
> +       CLK_DIV(BM1880_CLK_DIV_0_RV, "clk_div_0_rv", &bm1880_pll_clks[1].hw,
> +               BM1880_CLK_DIV12, 16, 5, 1, bm1880_div_table_0, 0),
> +       CLK_DIV(BM1880_CLK_DIV_1_RV, "clk_div_1_rv", &bm1880_pll_clks[2].hw,
> +               BM1880_CLK_DIV13, 16, 5, 1, bm1880_div_table_0, 0),
> +       CLK_DIV(BM1880_CLK_DIV_UART_500M, "clk_div_uart_500m", &bm1880_pll_clks[2].hw,
> +               BM1880_CLK_DIV15, 16, 7, 3, bm1880_div_table_1, 0),
> +       CLK_DIV(BM1880_CLK_DIV_0_AXI1, "clk_div_0_axi1", &bm1880_pll_clks[0].hw,
> +               BM1880_CLK_DIV21, 16, 5, 2, bm1880_div_table_0,
> +               CLK_IS_CRITICAL),
> +       CLK_DIV(BM1880_CLK_DIV_1_AXI1, "clk_div_1_axi1", &bm1880_pll_clks[2].hw,
> +               BM1880_CLK_DIV22, 16, 5, 3, bm1880_div_table_0,
> +               CLK_IS_CRITICAL),
> +       CLK_DIV(BM1880_CLK_DIV_0_AXI6, "clk_div_0_axi6", &bm1880_pll_clks[2].hw,
> +               BM1880_CLK_DIV27, 16, 5, 15, bm1880_div_table_0,
> +               CLK_IS_CRITICAL),
> +       CLK_DIV(BM1880_CLK_DIV_1_AXI6, "clk_div_1_axi6", &bm1880_pll_clks[0].hw,
> +               BM1880_CLK_DIV28, 16, 5, 11, bm1880_div_table_0,
> +               CLK_IS_CRITICAL),
> +       CLK_DIV(BM1880_CLK_DIV_12M_USB, "clk_div_12m_usb", &bm1880_pll_clks[2].hw,
> +               BM1880_CLK_DIV18, 16, 7, 125, bm1880_div_table_1, 0),
> +};
> +
> +/*
> + * Clocks marked as CRITICAL are all needed for the proper functioning
> + * of the SoC.
> + */
> +static struct bm1880_composite_clock bm1880_composite_clks[] = {
> +       GATE_MUX(BM1880_CLK_A53, "clk_a53", clk_a53_parents,
> +                BM1880_CLK_ENABLE0, 0, BM1880_CLK_SELECT, 0,
> +                CLK_IS_CRITICAL),
> +       GATE_DIV(BM1880_CLK_50M_A53, "clk_50m_a53", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 1, BM1880_CLK_DIV0, 16, 5, 30,
> +                bm1880_div_table_0, CLK_IS_CRITICAL),
> +       GATE_DIV(BM1880_CLK_EFUSE, "clk_efuse", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 5, BM1880_CLK_DIV1, 16, 7, 60,
> +                bm1880_div_table_1, 0),
> +       GATE_DIV(BM1880_CLK_EMMC, "clk_emmc", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 8, BM1880_CLK_DIV2, 16, 5, 15,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_100K_EMMC, "clk_100k_emmc", "clk_div_12m_usb",
> +                BM1880_CLK_ENABLE0, 9, BM1880_CLK_DIV3, 16, 8, 120,
> +                bm1880_div_table_2, 0),
> +       GATE_DIV(BM1880_CLK_SD, "clk_sd", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 11, BM1880_CLK_DIV4, 16, 5, 15,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_100K_SD, "clk_100k_sd", "clk_div_12m_usb",
> +                BM1880_CLK_ENABLE0, 12, BM1880_CLK_DIV5, 16, 8, 120,
> +                bm1880_div_table_2, 0),
> +       GATE_DIV(BM1880_CLK_500M_ETH0, "clk_500m_eth0", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 13, BM1880_CLK_DIV6, 16, 5, 3,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_500M_ETH1, "clk_500m_eth1", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 15, BM1880_CLK_DIV7, 16, 5, 3,
> +                bm1880_div_table_0, 0),
> +       /* Don't gate GPIO clocks as it is not owned by the GPIO driver */
> +       GATE_DIV(BM1880_CLK_GPIO_DB, "clk_gpio_db", "clk_div_12m_usb",
> +                BM1880_CLK_ENABLE0, 20, BM1880_CLK_DIV8, 16, 16, 120,
> +                bm1880_div_table_4, CLK_IGNORE_UNUSED),
> +       GATE_DIV(BM1880_CLK_SDMA_AUD, "clk_sdma_aud", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 24, BM1880_CLK_DIV9, 16, 7, 61,
> +                bm1880_div_table_1, 0),
> +       GATE_DIV(BM1880_CLK_JPEG_AXI, "clk_jpeg_axi", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 28, BM1880_CLK_DIV10, 16, 5, 4,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_NF, "clk_nf", "clk_fpll",
> +                BM1880_CLK_ENABLE0, 31, BM1880_CLK_DIV11, 16, 5, 30,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_TPU_AXI, "clk_tpu_axi", "clk_spll",
> +                BM1880_CLK_ENABLE1, 3, BM1880_CLK_DIV14, 16, 5, 1,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_125M_USB, "clk_125m_usb", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 9, BM1880_CLK_DIV16, 16, 5, 12,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_33K_USB, "clk_33k_usb", "clk_div_12m_usb",
> +                BM1880_CLK_ENABLE1, 10, BM1880_CLK_DIV17, 16, 9, 363,
> +                bm1880_div_table_3, 0),
> +       GATE_DIV(BM1880_CLK_VIDEO_AXI, "clk_video_axi", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 13, BM1880_CLK_DIV19, 16, 5, 4,
> +                bm1880_div_table_0, 0),
> +       GATE_DIV(BM1880_CLK_VPP_AXI, "clk_vpp_axi", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 14, BM1880_CLK_DIV20, 16, 5, 4,
> +                bm1880_div_table_0, 0),
> +       GATE_MUX(BM1880_CLK_AXI1, "clk_axi1", clk_axi1_parents,
> +                BM1880_CLK_ENABLE1, 15, BM1880_CLK_SELECT, 2,
> +                CLK_IS_CRITICAL),
> +       GATE_DIV(BM1880_CLK_AXI2, "clk_axi2", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 17, BM1880_CLK_DIV23, 16, 5, 3,
> +                bm1880_div_table_0, CLK_IS_CRITICAL),
> +       GATE_DIV(BM1880_CLK_AXI3, "clk_axi3", "clk_mux_rv",
> +                BM1880_CLK_ENABLE1, 18, BM1880_CLK_DIV24, 16, 5, 2,
> +                bm1880_div_table_0, CLK_IS_CRITICAL),
> +       GATE_DIV(BM1880_CLK_AXI4, "clk_axi4", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 19, BM1880_CLK_DIV25, 16, 5, 6,
> +                bm1880_div_table_0, CLK_IS_CRITICAL),
> +       GATE_DIV(BM1880_CLK_AXI5, "clk_axi5", "clk_fpll",
> +                BM1880_CLK_ENABLE1, 20, BM1880_CLK_DIV26, 16, 5, 15,
> +                bm1880_div_table_0, CLK_IS_CRITICAL),
> +};
> +
> +static unsigned long bm1880_pll_rate_calc(u32 regval, unsigned long parent_rate)
> +{
> +       u32 fbdiv, fref, refdiv;
> +       u32 postdiv1, postdiv2;
> +       unsigned long rate, numerator, denominator;
> +
> +       fbdiv = (regval >> 16) & 0xfff;
> +       fref = parent_rate;
> +       refdiv = regval & 0x1f;
> +       postdiv1 = (regval >> 8) & 0x7;
> +       postdiv2 = (regval >> 12) & 0x7;
> +
> +       numerator = parent_rate * fbdiv;
> +       denominator = refdiv * postdiv1 * postdiv2;
> +       do_div(numerator, denominator);
> +       rate = numerator;
> +
> +       return rate;
> +}
> +
> +static unsigned long bm1880_pll_recalc_rate(struct clk_hw *hw,
> +                                           unsigned long parent_rate)
> +{
> +       struct bm1880_pll_hw_clock *pll_hw = to_bm1880_pll_clk(hw);
> +       unsigned long rate;
> +       u32 regval;
> +
> +       regval = readl(pll_hw->base + pll_hw->pll.reg);
> +       rate = bm1880_pll_rate_calc(regval, parent_rate);
> +
> +       return rate;
> +}
> +
> +static const struct clk_ops bm1880_pll_ops = {
> +       .recalc_rate    = bm1880_pll_recalc_rate,
> +};
> +
> +static struct clk_hw *bm1880_clk_register_pll(struct bm1880_pll_hw_clock *pll_clk,
> +                                             void __iomem *sys_base)
> +{
> +       struct clk_hw *hw;
> +       int err;
> +
> +       pll_clk->base = sys_base;
> +       hw = &pll_clk->hw;
> +
> +       err = clk_hw_register(NULL, hw);
> +       if (err)
> +               return ERR_PTR(err);
> +
> +       return hw;
> +}
> +
> +static void bm1880_clk_unregister_pll(struct clk_hw *hw)
> +{
> +       struct bm1880_pll_hw_clock *pll_hw = to_bm1880_pll_clk(hw);
> +
> +       clk_hw_unregister(hw);
> +       kfree(pll_hw);
> +}
> +
> +static int bm1880_clk_register_plls(struct bm1880_pll_hw_clock *clks,
> +                            int num_clks, struct bm1880_clock_data *data)
> +{
> +       struct clk_hw *hw;
> +       void __iomem *pll_base = data->pll_base;
> +       int i;
> +
> +       for (i = 0; i < num_clks; i++) {
> +               struct bm1880_pll_hw_clock *bm1880_clk = &clks[i];
> +
> +               hw = bm1880_clk_register_pll(bm1880_clk, pll_base);
> +               if (IS_ERR(hw)) {
> +                       pr_err("%s: failed to register clock %s\n",
> +                              __func__, bm1880_clk->pll.name);
> +                       goto err_clk;
> +               }
> +
> +               data->clk_data->hws[clks[i].pll.id] = hw;
> +       }
> +
> +       return 0;
> +
> +err_clk:
> +       while (i--)
> +               bm1880_clk_unregister_pll(data->clk_data->hws[clks[i].pll.id]);
> +
> +       return PTR_ERR(hw);
> +}
> +
> +static int bm1880_clk_register_mux(const struct bm1880_mux_clock *clks,
> +                           int num_clks, struct bm1880_clock_data *data)
> +{
> +       struct clk_hw *hw;
> +       void __iomem *sys_base = data->sys_base;
> +       int i;
> +
> +       for (i = 0; i < num_clks; i++) {
> +               hw = clk_hw_register_mux(NULL, clks[i].name,
> +                                        clks[i].parents,
> +                                        clks[i].num_parents,
> +                                        clks[i].flags,
> +                                        sys_base + clks[i].reg,
> +                                        clks[i].shift, 1, 0,
> +                                        &bm1880_clk_lock);
> +               if (IS_ERR(hw)) {
> +                       pr_err("%s: failed to register clock %s\n",
> +                              __func__, clks[i].name);
> +                       goto err_clk;
> +               }
> +
> +               data->clk_data->hws[clks[i].id] = hw;
> +       }
> +
> +       return 0;
> +
> +err_clk:
> +       while (i--)
> +               clk_hw_unregister_mux(data->clk_data->hws[clks[i].id]);
> +
> +       return PTR_ERR(hw);
> +}
> +
> +static unsigned long bm1880_clk_div_recalc_rate(struct clk_hw *hw,
> +                                               unsigned long parent_rate)
> +{
> +       struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> +       struct bm1880_div_clock *div = &div_hw->div;
> +       void __iomem *reg_addr = div_hw->base + div->reg;
> +       unsigned int val;
> +       unsigned long rate;
> +
> +       if (!(readl(reg_addr) & BIT(3))) {
> +               val = div->initval;
> +       } else {
> +               val = readl(reg_addr) >> div->shift;
> +               val &= clk_div_mask(div->width);
> +       }
> +
> +       rate = divider_recalc_rate(hw, parent_rate, val, div->table,
> +                                  div->flags, div->width);
> +
> +       return rate;
> +}
> +
> +static long bm1880_clk_div_round_rate(struct clk_hw *hw, unsigned long rate,
> +                                     unsigned long *prate)
> +{
> +       struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> +       struct bm1880_div_clock *div = &div_hw->div;
> +       void __iomem *reg_addr = div_hw->base + div->reg;
> +
> +       if (div->flags & CLK_DIVIDER_READ_ONLY) {
> +               u32 val;
> +
> +               val = readl(reg_addr) >> div->shift;
> +               val &= clk_div_mask(div->width);
> +
> +               return divider_ro_round_rate(hw, rate, prate, div->table,
> +                                            div->width, div->flags,
> +                                            val);
> +       }
> +
> +       return divider_round_rate(hw, rate, prate, div->table,
> +                                 div->width, div->flags);
> +}
> +
> +static int bm1880_clk_div_set_rate(struct clk_hw *hw, unsigned long rate,
> +                                  unsigned long parent_rate)
> +{
> +       struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> +       struct bm1880_div_clock *div = &div_hw->div;
> +       void __iomem *reg_addr = div_hw->base + div->reg;
> +       unsigned long flags = 0;
> +       int value;
> +       u32 val;
> +
> +       value = divider_get_val(rate, parent_rate, div->table,
> +                               div->width, div_hw->div.flags);
> +       if (value < 0)
> +               return value;
> +
> +       if (div_hw->lock)
> +               spin_lock_irqsave(div_hw->lock, flags);
> +       else
> +               __acquire(div_hw->lock);
> +
> +       if (div->flags & CLK_DIVIDER_HIWORD_MASK) {

Is this used by your driver? As far as I can recall it was a
rockchip/hisilicon specific thing that hasn't happened since.

> +               val = clk_div_mask(div->width) << (div_hw->div.shift + 16);
> +       } else {
> +               val = readl(reg_addr);
> +               val &= ~(clk_div_mask(div->width) << div_hw->div.shift);
> +       }
> +       val |= (u32)value << div->shift;
> +       writel(val, reg_addr);
> +
> +       if (div_hw->lock)
> +               spin_unlock_irqrestore(div_hw->lock, flags);
> +       else
> +               __release(div_hw->lock);
> +
> +       return 0;
> +}
> +
> +static const struct clk_ops bm1880_clk_div_ops = {
> +       .recalc_rate = bm1880_clk_div_recalc_rate,
> +       .round_rate = bm1880_clk_div_round_rate,
> +       .set_rate = bm1880_clk_div_set_rate,
> +};
> +
> +static struct clk_hw *bm1880_clk_register_div(struct bm1880_div_hw_clock *div_clk,
> +                                   void __iomem *sys_base)
> +{
> +       struct clk_hw *hw;
> +       int err;
> +
> +       div_clk->div.flags = CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO;
> +       div_clk->base = sys_base;
> +       div_clk->lock = &bm1880_clk_lock;
> +
> +       hw = &div_clk->hw;
> +       err = clk_hw_register(NULL, hw);
> +       if (err)
> +               return ERR_PTR(err);
> +
> +       return hw;
> +}
> +
> +static void bm1880_clk_unregister_div(struct clk_hw *hw)
> +{
> +       struct bm1880_div_hw_clock *div_hw = to_bm1880_div_clk(hw);
> +
> +       clk_hw_unregister(hw);
> +       kfree(div_hw);
> +}
> +
> +static int bm1880_clk_register_divs(struct bm1880_div_hw_clock *clks,
> +                            int num_clks, struct bm1880_clock_data *data)
> +{
> +       struct clk_hw *hw;
> +       void __iomem *sys_base = data->sys_base;
> +       int i;
> +
> +       for (i = 0; i < num_clks; i++) {
> +               struct bm1880_div_hw_clock *bm1880_clk = &clks[i];
> +
> +               hw = bm1880_clk_register_div(bm1880_clk, sys_base);
> +               if (IS_ERR(hw)) {
> +                       pr_err("%s: failed to register clock %s\n",
> +                              __func__, bm1880_clk->div.name);
> +                       goto err_clk;
> +               }
> +
> +               data->clk_data->hws[clks[i].div.id] = hw;

This line is overly complicated. Please use local variables.

> +       }
> +
> +       return 0;
> +
> +err_clk:
> +       while (i--)
> +               bm1880_clk_unregister_div(data->clk_data->hws[clks[i].div.id]);
> +
> +       return PTR_ERR(hw);
> +}
> +
> +static int bm1880_clk_register_gate(const struct bm1880_gate_clock *clks,
> +                            int num_clks, struct bm1880_clock_data *data)
> +{
> +       struct clk_hw *hw;
> +       void __iomem *sys_base = data->sys_base;
> +       int i;
> +
> +       for (i = 0; i < num_clks; i++) {
> +               hw = clk_hw_register_gate(NULL, clks[i].name,
> +                                         clks[i].parent,
> +                                         clks[i].flags,
> +                                         sys_base + clks[i].gate_reg,
> +                                         clks[i].gate_shift,
> +                                       0,
> +                                       &bm1880_clk_lock);

Weird tabs here.

> +               if (IS_ERR(hw)) {
> +                       pr_err("%s: failed to register clock %s\n",
> +                              __func__, clks[i].name);
> +                       goto err_clk;
> +               }
> +
> +               data->clk_data->hws[clks[i].id] = hw;
> +       }
> +
> +       return 0;
> +
> +err_clk:
> +       while (i--)
> +               clk_hw_unregister_gate(data->clk_data->hws[clks[i].id]);
> +
> +       return PTR_ERR(hw);
> +}
> +
> +static struct clk_hw *bm1880_clk_register_composite(struct bm1880_composite_clock *clks,
> +                                         void __iomem *sys_base)
> +{
> +       struct clk_hw *hw;
> +       struct clk_mux *mux = NULL;
> +       struct clk_gate *gate = NULL;
> +       struct bm1880_div_hw_clock *div_hws = NULL;
> +       struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *div_hw = NULL;
> +       const struct clk_ops *mux_ops = NULL, *gate_ops = NULL, *div_ops = NULL;
> +       const char * const *parent_names;
> +       const char *parent;
> +       int num_parents;
> +       int ret;
> +
> +       if (clks->mux_shift >= 0) {
> +               mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> +               if (!mux)
> +                       return ERR_PTR(-ENOMEM);
> +
> +               mux->reg = sys_base + clks->mux_reg;
> +               mux->mask = 1;
> +               mux->shift = clks->mux_shift;
> +               mux_hw = &mux->hw;
> +               mux_ops = &clk_mux_ops;
> +               mux->lock = &bm1880_clk_lock;
> +
> +               parent_names = clks->parents;
> +               num_parents = clks->num_parents;
> +       } else {
> +               parent = clks->parent;
> +               parent_names = &parent;
> +               num_parents = 1;
> +       }
> +
> +       if (clks->gate_shift >= 0) {
> +               gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> +               if (!gate) {
> +                       ret = -ENOMEM;
> +                       goto err_out;
> +               }
> +
> +               gate->reg = sys_base + clks->gate_reg;
> +               gate->bit_idx = clks->gate_shift;
> +               gate->lock = &bm1880_clk_lock;
> +
> +               gate_hw = &gate->hw;
> +               gate_ops = &clk_gate_ops;
> +       }
> +
> +       if (clks->div_shift >= 0) {
> +               div_hws = kzalloc(sizeof(*div_hws), GFP_KERNEL);
> +               if (!div_hws) {
> +                       ret = -ENOMEM;
> +                       goto err_out;
> +               }
> +
> +               div_hws->base = sys_base;
> +               div_hws->div.reg = clks->div_reg;
> +               div_hws->div.shift = clks->div_shift;
> +               div_hws->div.width = clks->div_width;
> +               div_hws->div.table = clks->table;
> +               div_hws->div.initval = clks->div_initval;
> +               div_hws->lock = &bm1880_clk_lock;
> +               div_hws->div.flags = CLK_DIVIDER_ONE_BASED |
> +                                    CLK_DIVIDER_ALLOW_ZERO;
> +
> +               div_hw = &div_hws->hw;
> +               div_ops = &bm1880_clk_div_ops;
> +       }
> +
> +       hw = clk_hw_register_composite(NULL, clks->name, parent_names,
> +                                      num_parents, mux_hw, mux_ops, div_hw,
> +                                      div_ops, gate_hw, gate_ops,
> +                                      clks->flags);
> +

All of these need to be removed on probe failure. Any plans to do so?

> +       if (IS_ERR(hw)) {
> +               ret = PTR_ERR(hw);
> +               goto err_out;
> +       }
> +
> +       return hw;
> +
> +err_out:
> +       kfree(div_hws);
> +       kfree(gate);
> +       kfree(mux);
> +
> +       return ERR_PTR(ret);
> +}
> +
> +static int bm1880_clk_register_composites(struct bm1880_composite_clock *clks,
> +                                  int num_clks, struct bm1880_clock_data *data)
> +{
> +       struct clk_hw *hw;
> +       void __iomem *sys_base = data->sys_base;
> +       int i;
> +
> +       for (i = 0; i < num_clks; i++) {
> +               struct bm1880_composite_clock *bm1880_clk = &clks[i];
> +
> +               hw = bm1880_clk_register_composite(bm1880_clk, sys_base);
> +               if (IS_ERR(hw)) {
> +                       pr_err("%s: failed to register clock %s\n",
> +                              __func__, bm1880_clk->name);
> +                       goto err_clk;
> +               }
> +
> +               data->clk_data->hws[clks[i].id] = hw;
> +       }
> +
> +       return 0;
> +
> +err_clk:
> +       while (i--)
> +               clk_hw_unregister_composite(data->clk_data->hws[clks[i].id]);
> +
> +       return PTR_ERR(hw);
> +}
> +
> +static int bm1880_clk_probe(struct platform_device *pdev)
> +{
> +       struct bm1880_clock_data *clk_data;
> +       void __iomem *pll_base, *sys_base;
> +       struct device_node *np = pdev->dev.of_node;
> +       struct device *dev = &pdev->dev;
> +       struct resource *res;
> +       struct clk_hw_onecell_data *clk_hw_data;
> +       int num_clks, i;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       pll_base = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(pll_base))
> +               return PTR_ERR(pll_base);
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +       sys_base = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(sys_base))
> +               return PTR_ERR(sys_base);
> +
> +       clk_data = devm_kzalloc(dev, sizeof(*clk_data), GFP_KERNEL);
> +       if (!clk_data)
> +               return -ENOMEM;
> +
> +       clk_data->pll_base = pll_base;
> +       clk_data->sys_base = sys_base;
> +
> +       num_clks = ARRAY_SIZE(bm1880_pll_clks) +
> +                  ARRAY_SIZE(bm1880_div_clks) +
> +                  ARRAY_SIZE(bm1880_mux_clks) +
> +                  ARRAY_SIZE(bm1880_composite_clks) +
> +                  ARRAY_SIZE(bm1880_gate_clks);
> +
> +       clk_hw_data = devm_kzalloc(&pdev->dev, struct_size(clk_hw_data, hws,
> +                                         num_clks), GFP_KERNEL);
> +       if (!clk_hw_data)
> +               return -ENOMEM;
> +
> +       clk_data->clk_data = clk_hw_data;
> +
> +       for (i = 0; i < num_clks; i++)
> +               clk_data->clk_data->hws[i] = ERR_PTR(-ENOENT);
> +
> +       clk_data->clk_data->num = num_clks;
> +
> +       bm1880_clk_register_plls(bm1880_pll_clks,
> +                                ARRAY_SIZE(bm1880_pll_clks),
> +                                clk_data);
> +
> +       bm1880_clk_register_divs(bm1880_div_clks,
> +                                ARRAY_SIZE(bm1880_div_clks),
> +                                clk_data);
> +
> +       bm1880_clk_register_mux(bm1880_mux_clks,
> +                               ARRAY_SIZE(bm1880_mux_clks),
> +                               clk_data);
> +
> +       bm1880_clk_register_composites(bm1880_composite_clks,
> +                                      ARRAY_SIZE(bm1880_composite_clks),
> +                                      clk_data);
> +
> +       bm1880_clk_register_gate(bm1880_gate_clks,
> +                                ARRAY_SIZE(bm1880_gate_clks),
> +                                clk_data);
> +
> +       return of_clk_add_hw_provider(np, of_clk_hw_onecell_get,

Can you use devm_of_clk_add_hw_provider()?

> +                                     clk_data->clk_data);
> +}

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] clk: at91: allow 24 Mhz clock as input for PLL
From: Stephen Boyd @ 2019-09-18  5:00 UTC (permalink / raw)
  To: Eugen.Hristev, alexandre.belloni, linux-arm-kernel, linux-clk,
	linux-kernel, mturquette
  Cc: Eugen.Hristev
In-Reply-To: <1568183622-7858-1-git-send-email-eugen.hristev@microchip.com>

Quoting Eugen.Hristev@microchip.com (2019-09-10 23:39:20)
> From: Eugen Hristev <eugen.hristev@microchip.com>
> 
> The PLL input range needs to be able to allow 24 Mhz crystal as input
> Update the range accordingly in plla characteristics struct
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---

Applied to clk-next


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V2 2/2] mm/pgtable/debug: Add test validating architecture page table helpers
From: Anshuman Khandual @ 2019-09-18  5:04 UTC (permalink / raw)
  To: Christophe Leroy, linux-mm
  Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
	Heiko Carstens, Michal Hocko, Dave Hansen, Paul Mackerras,
	sparclinux, Thomas Gleixner, linux-s390, Jason Gunthorpe,
	Michael Ellerman, x86, Russell King - ARM Linux, Matthew Wilcox,
	Steven Price, Tetsuo Handa, Gerald Schaefer, linux-snps-arc,
	Kees Cook, Masahiro Yamada, Mark Brown, Kirill A . Shutemov,
	Dan Williams, Vlastimil Babka, linux-arm-kernel,
	Sri Krishna chowdary, Ard Biesheuvel, Greg Kroah-Hartman,
	linux-mips, Ralf Baechle, linux-kernel, Paul Burton,
	Mike Rapoport, Vineet Gupta, Martin Schwidefsky, Andrew Morton,
	linuxppc-dev, David S. Miller
In-Reply-To: <95ed9d92-dd43-4c45-2e52-738aed7f2fb5@c-s.fr>



On 09/13/2019 03:31 PM, Christophe Leroy wrote:
> 
> 
> Le 13/09/2019 à 11:02, Anshuman Khandual a écrit :
>>
>>>> +#if !defined(__PAGETABLE_PMD_FOLDED) && !defined(__ARCH_HAS_4LEVEL_HACK)
>>>
>>> #ifdefs have to be avoided as much as possible, see below
>>
>> Yeah but it has been bit difficult to avoid all these $ifdef because of the
>> availability (or lack of it) for all these pgtable helpers in various config
>> combinations on all platforms.
> 
> As far as I can see these pgtable helpers should exist everywhere at least via asm-generic/ files.

But they might not actually do the right thing.

> 
> Can you spot a particular config which fails ?

Lets consider the following example (after removing the $ifdefs around it)
which though builds successfully but fails to pass the intended test. This
is with arm64 config 4K pages sizes with 39 bits VA space which ends up
with a 3 level page table arrangement.

static void __init p4d_clear_tests(p4d_t *p4dp)
{
        p4d_t p4d = READ_ONCE(*p4dp);

        p4d = __p4d(p4d_val(p4d) | RANDOM_ORVALUE);
        WRITE_ONCE(*p4dp, p4d);
        p4d_clear(p4dp);
        p4d = READ_ONCE(*p4dp);
        WARN_ON(!p4d_none(p4d));
}

The following test hits an error at WARN_ON(!p4d_none(p4d))

[   16.757333] ------------[ cut here ]------------
[   16.758019] WARNING: CPU: 11 PID: 1 at mm/arch_pgtable_test.c:187 arch_pgtable_tests_init+0x24c/0x474
[   16.759455] Modules linked in:
[   16.759952] CPU: 11 PID: 1 Comm: swapper/0 Not tainted 5.3.0-next-20190916-00005-g61c218153bb8-dirty #222
[   16.761449] Hardware name: linux,dummy-virt (DT)
[   16.762185] pstate: 00400005 (nzcv daif +PAN -UAO)
[   16.762964] pc : arch_pgtable_tests_init+0x24c/0x474
[   16.763750] lr : arch_pgtable_tests_init+0x174/0x474
[   16.764534] sp : ffffffc011d7bd50
[   16.765065] x29: ffffffc011d7bd50 x28: ffffffff1756bac0 
[   16.765908] x27: ffffff85ddaf3000 x26: 00000000000002e8 
[   16.766767] x25: ffffffc0111ce000 x24: ffffff85ddaf32e8 
[   16.767606] x23: ffffff85ddaef278 x22: 00000045cc844000 
[   16.768445] x21: 000000065daef003 x20: ffffffff17540000 
[   16.769283] x19: ffffff85ddb60000 x18: 0000000000000014 
[   16.770122] x17: 00000000980426bb x16: 00000000698594c6 
[   16.770976] x15: 0000000066e25a88 x14: 0000000000000000 
[   16.771813] x13: ffffffff17540000 x12: 000000000000000a 
[   16.772651] x11: ffffff85fcfd0a40 x10: 0000000000000001 
[   16.773488] x9 : 0000000000000008 x8 : ffffffc01143ab26 
[   16.774336] x7 : 0000000000000000 x6 : 0000000000000000 
[   16.775180] x5 : 0000000000000000 x4 : 0000000000000000 
[   16.776018] x3 : ffffffff1756bbe8 x2 : 000000065daeb003 
[   16.776856] x1 : 000000000065daeb x0 : fffffffffffff000 
[   16.777693] Call trace:
[   16.778092]  arch_pgtable_tests_init+0x24c/0x474
[   16.778843]  do_one_initcall+0x74/0x1b0
[   16.779458]  kernel_init_freeable+0x1cc/0x290
[   16.780151]  kernel_init+0x10/0x100
[   16.780710]  ret_from_fork+0x10/0x18
[   16.781282] ---[ end trace 042e6c40c0a3b038 ]---

On arm64 (4K page size|39 bits VA|3 level page table)

#elif CONFIG_PGTABLE_LEVELS == 3	/* Applicable here */
#define __ARCH_USE_5LEVEL_HACK
#include <asm-generic/pgtable-nopud.h>

Which pulls in 

#include <asm-generic/pgtable-nop4d-hack.h>

which pulls in

#include <asm-generic/5level-fixup.h>

which defines

static inline int p4d_none(p4d_t p4d)
{
        return 0;
}

which will invariably trigger WARN_ON(!p4d_none(p4d)).

Similarly for next test p4d_populate_tests() which will always be
successful because p4d_bad() invariably returns negative.

static inline int p4d_bad(p4d_t p4d)
{
        return 0;
}

static void __init p4d_populate_tests(struct mm_struct *mm, p4d_t *p4dp,
                                      pud_t *pudp)
{
        p4d_t p4d;

        /*
         * This entry points to next level page table page.
         * Hence this must not qualify as p4d_bad().
         */
        pud_clear(pudp);
        p4d_clear(p4dp);
        p4d_populate(mm, p4dp, pudp);
        p4d = READ_ONCE(*p4dp);
        WARN_ON(p4d_bad(p4d));
}

We should not run these tests for the above config because they are
not applicable and will invariably produce same result.

> 
>>
>>>
> 
> [...]
> 
>>>> +#if !defined(__PAGETABLE_PUD_FOLDED) && !defined(__ARCH_HAS_5LEVEL_HACK)
>>>
>>> The same can be done here.
>>
>> IIRC not only the page table helpers but there are data types (pxx_t) which
>> were not present on various configs and these wrappers help prevent build
>> failures. Any ways will try and see if this can be improved further. But
>> meanwhile if you have some suggestions, please do let me know.
> 
> pgt_t and pmd_t are everywhere I guess.
> then pud_t and p4d_t have fallbacks in asm-generic files.

Lets take another example where it fails to compile. On arm64 with 16K
page size, 48 bits VA, 4 level page table arrangement in the following
test, pgd_populate() does not have the required signature.

static void pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp, p4d_t *p4dp)
{
        pgd_t pgd;

        if (mm_p4d_folded(mm))
                return;

       /*
         * This entry points to next level page table page.
         * Hence this must not qualify as pgd_bad().
         */
        p4d_clear(p4dp);
        pgd_clear(pgdp);
        pgd_populate(mm, pgdp, p4dp);
        pgd = READ_ONCE(*pgdp);
        WARN_ON(pgd_bad(pgd));
}

mm/arch_pgtable_test.c: In function ‘pgd_populate_tests’:
mm/arch_pgtable_test.c:254:25: error: passing argument 3 of ‘pgd_populate’ from incompatible pointer type [-Werror=incompatible-pointer-types]
  pgd_populate(mm, pgdp, p4dp);
                         ^~~~
In file included from mm/arch_pgtable_test.c:27:0:
./arch/arm64/include/asm/pgalloc.h:81:20: note: expected ‘pud_t * {aka struct <anonymous> *}’ but argument is of type ‘pgd_t * {aka struct <anonymous> *}’
 static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgdp, pud_t *pudp)

The build failure is because p4d_t * maps to pgd_t * but the applicable
(it does not fallback on generic ones) pgd_populate() expects a pud_t *.

Except for archs which have 5 level page able, pgd_populate() always accepts
lower level page table pointers as the last argument as they dont have that
many levels.

arch/x86/include/asm/pgalloc.h:static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, p4d_t *p4d)
arch/s390/include/asm/pgalloc.h:static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, p4d_t *p4d)

But others

arch/arm64/include/asm/pgalloc.h:static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgdp, pud_t *pudp)
arch/m68k/include/asm/motorola_pgalloc.h:static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, pmd_t *pmd)
arch/mips/include/asm/pgalloc.h:static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, pud_t *pud)
arch/powerpc/include/asm/book3s/64/pgalloc.h:static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, pud_t *pud)

I remember going through all these combinations before arriving at the
current state of #ifdef exclusions. Probably, to solved this all platforms
have to define pxx_populate() helpers assuming they support 5 level page
table.

> 
> So it shouldn't be an issue. Maybe if a couple of arches miss them, the best would be to fix the arches, since that's the purpose of your testsuite isn't it ?

The run time failures as explained previously is because of the folding which
needs to be protected as they are not even applicable. The compile time
failures are because pxx_populate() signatures are platform specific depending
on how many page table levels they really support.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [linux-sunxi] [PATCH] clk: sunxi-ng: h6: Use sigma-delta modulation for audio PLL
From: Jernej Škrabec @ 2019-09-18  5:21 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Stephen Boyd, Mike Turquette, linux-kernel, Maxime Ripard,
	linux-sunxi, linux-clk, linux-arm-kernel
In-Reply-To: <CAGb2v640R7edA3EJvC=aJQZXGcfqot50O3-PFyrYj767pUEYrQ@mail.gmail.com>

Dne torek, 17. september 2019 ob 08:54:08 CEST je Chen-Yu Tsai napisal(a):
> On Sat, Sep 14, 2019 at 9:51 PM Jernej Skrabec <jernej.skrabec@siol.net> 
wrote:
> > Audio devices needs exact clock rates in order to correctly reproduce
> > the sound. Until now, only integer factors were used to configure H6
> > audio PLL which resulted in inexact rates. Fix that by adding support
> > for fractional factors using sigma-delta modulation look-up table. It
> > contains values for two most commonly used audio base frequencies.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> > 
> >  drivers/clk/sunxi-ng/ccu-sun50i-h6.c | 21 +++++++++++++++------
> >  1 file changed, 15 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> > b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c index d89353a3cdec..ed6338d74474
> > 100644
> > --- a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> > @@ -203,12 +203,21 @@ static struct ccu_nkmp pll_hsic_clk = {
> > 
> >   * hardcode it to match with the clock names.
> >   */
> >  
> >  #define SUN50I_H6_PLL_AUDIO_REG                0x078
> > 
> > +
> > +static struct ccu_sdm_setting pll_audio_sdm_table[] = {
> > +       { .rate = 541900800, .pattern = 0xc001288d, .m = 1, .n = 22 },
> > +       { .rate = 589824000, .pattern = 0xc00126e9, .m = 1, .n = 24 },
> > +};
> > +
> > 
> >  static struct ccu_nm pll_audio_base_clk = {
> >  
> >         .enable         = BIT(31),
> >         .lock           = BIT(28),
> >         .n              = _SUNXI_CCU_MULT_MIN(8, 8, 12),
> >         .m              = _SUNXI_CCU_DIV(1, 1), /* input divider */
> > 
> > +       .sdm            = _SUNXI_CCU_SDM(pll_audio_sdm_table,
> > +                                        BIT(24), 0x178, BIT(31)),
> > 
> >         .common         = {
> > 
> > +               .features       = CCU_FEATURE_SIGMA_DELTA_MOD,
> > 
> >                 .reg            = 0x078,
> >                 .hw.init        = CLK_HW_INIT("pll-audio-base", "osc24M",
> >                 
> >                                               &ccu_nm_ops,
> > 
> > @@ -753,12 +762,12 @@ static const struct clk_hw *clk_parent_pll_audio[] =
> > {> 
> >  };
> >  
> >  /*
> > 
> > - * The divider of pll-audio is fixed to 8 now, as pll-audio-4x has a
> > - * fixed post-divider 2.
> > + * The divider of pll-audio is fixed to 24 for now, so 24576000 and
> > 22579200 + * rates can be set exactly in conjunction with sigma-delta
> > modulation.> 
> >   */
> >  
> >  static CLK_FIXED_FACTOR_HWS(pll_audio_clk, "pll-audio",
> >  
> >                             clk_parent_pll_audio,
> > 
> > -                           8, 1, CLK_SET_RATE_PARENT);
> > +                           24, 1, CLK_SET_RATE_PARENT);
> > 
> >  static CLK_FIXED_FACTOR_HWS(pll_audio_2x_clk, "pll-audio-2x",
> >  
> >                             clk_parent_pll_audio,
> >                             4, 1, CLK_SET_RATE_PARENT);
> 
> You need to fix the factors for the other two outputs as well, since all
> three are derived from pll-audio-base.

Fix how? pll-audio-2x and pll-audio-4x clocks have fixed divider in regards to 
pll-audio-base, while pll-audio has not. Unless you mean changing their name?

Best regards,
Jernej

> 
> ChenYu
> 
> > @@ -1215,12 +1224,12 @@ static int sun50i_h6_ccu_probe(struct
> > platform_device *pdev)> 
> >         }
> >         
> >         /*
> > 
> > -        * Force the post-divider of pll-audio to 8 and the output divider
> > -        * of it to 1, to make the clock name represents the real
> > frequency. +        * Force the post-divider of pll-audio to 12 and the
> > output divider +        * of it to 2, so 24576000 and 22579200 rates can
> > be set exactly.> 
> >          */
> >         
> >         val = readl(reg + SUN50I_H6_PLL_AUDIO_REG);
> >         val &= ~(GENMASK(21, 16) | BIT(0));
> > 
> > -       writel(val | (7 << 16), reg + SUN50I_H6_PLL_AUDIO_REG);
> > +       writel(val | (11 << 16) | BIT(0), reg + SUN50I_H6_PLL_AUDIO_REG);
> > 
> >         /*
> >         
> >          * First clock parent (osc32K) is unusable for CEC. But since
> >          there
> > 
> > --
> > 2.23.0
> > 
> > --
> > You received this message because you are subscribed to the Google Groups
> > "linux-sunxi" group. To unsubscribe from this group and stop receiving
> > emails from it, send an email to
> > linux-sunxi+unsubscribe@googlegroups.com. To view this discussion on the
> > web, visit
> > https://groups.google.com/d/msgid/linux-sunxi/20190914135100.327412-1-jer
> > nej.skrabec%40siol.net.





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V6 1/2] dt-bindings: mailbox: add binding doc for the ARM SMC/HVC mailbox
From: Jassi Brar @ 2019-09-18  5:27 UTC (permalink / raw)
  To: Andre Przywara
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, Peng Fan,
	f.fainelli@gmail.com, linux-kernel@vger.kernel.org,
	robh+dt@kernel.org, dl-linux-imx, sudeep.holla@arm.com,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190917183115.3e40180f@donnerap.cambridge.arm.com>

On Tue, Sep 17, 2019 at 12:31 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Mon, 16 Sep 2019 09:44:37 +0000
> Peng Fan <peng.fan@nxp.com> wrote:
>
> Hi,
>
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The ARM SMC/HVC mailbox binding describes a firmware interface to trigger
> > actions in software layers running in the EL2 or EL3 exception levels.
> > The term "ARM" here relates to the SMC instruction as part of the ARM
> > instruction set, not as a standard endorsed by ARM Ltd.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  .../devicetree/bindings/mailbox/arm-smc.yaml       | 96 ++++++++++++++++++++++
> >  1 file changed, 96 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.yaml b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > new file mode 100644
> > index 000000000000..bf01bec035fc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > @@ -0,0 +1,96 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mailbox/arm-smc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ARM SMC Mailbox Interface
> > +
> > +maintainers:
> > +  - Peng Fan <peng.fan@nxp.com>
> > +
> > +description: |
> > +  This mailbox uses the ARM smc (secure monitor call) and hvc (hypervisor
>
> I think "or" instead of "and" is less confusing.
>
> > +  call) instruction to trigger a mailbox-connected activity in firmware,
> > +  executing on the very same core as the caller. The value of r0/w0/x0
> > +  the firmware returns after the smc call is delivered as a received
> > +  message to the mailbox framework, so synchronous communication can be
> > +  established. The exact meaning of the action the mailbox triggers as
> > +  well as the return value is defined by their users and is not subject
> > +  to this binding.
> > +
> > +  One use case of this mailbox is the SCMI interface, which uses shared
>
>      One example use case of this mailbox ...
> (to make it more obvious that it's not restricted to this)
>
> > +  memory to transfer commands and parameters, and a mailbox to trigger a
> > +  function call. This allows SoCs without a separate management processor
> > +  (or when such a processor is not available or used) to use this
> > +  standardized interface anyway.
> > +
> > +  This binding describes no hardware, but establishes a firmware interface.
> > +  Upon receiving an SMC using one of the described SMC function identifiers,
>
>                              ... the described SMC function identifier,
>
> > +  the firmware is expected to trigger some mailbox connected functionality.
> > +  The communication follows the ARM SMC calling convention.
> > +  Firmware expects an SMC function identifier in r0 or w0. The supported
> > +  identifiers are passed from consumers,
>
>      identifier
>
> "passed from consumers": How? Where?
> But I want to repeat: We should not allow this.
> This is a binding for a mailbox controller driver, not a generic firmware backdoor.
>
Exactly. The mailbox controller here is the  SMC/HVC instruction,
which needs 9 arguments to work. The fact that the fist argument is
always going to be same on a platform is just the way we use this
instruction.

> We should be as strict as possible to avoid any security issues.
>
Any example of such a security issue?

> The firmware certainly knows the function ID it implements. The firmware controls the DT. So it is straight-forward to put the ID into the DT. The firmware could even do this at boot time, dynamically, before passing on the DT to the non-secure world (bootloader or kernel).
>
> What would be the use case of this functionality?
>
At least for flexibility and consistency.

> > or listed in the the arm,func-ids
>
>                        arm,func-id
>
> > +  properties as described below. The firmware can return one value in
>
>      property
>
> > +  the first SMC result register, it is expected to be an error value,
> > +  which shall be propagated to the mailbox client.
> > +
> > +  Any core which supports the SMC or HVC instruction can be used, as long
> > +  as a firmware component running in EL3 or EL2 is handling these calls.
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - description:
> > +          For implementations using ARM SMC instruction.
> > +        const: arm,smc-mbox
> > +
> > +      - description:
> > +          For implementations using ARM HVC instruction.
> > +        const: arm,hvc-mbox
>
> I am not particularly happy with this, but well ...
>
> > +
> > +  "#mbox-cells":
> > +    const: 1
>
> Why is this "1"? What is this number used for? It used to be the channel ID, but since you are describing a single channel controller only, this should be 0 now.
>
Yes. I overlooked it and actually queued the patch for pull request.
But I think the bindings should not carry a 'fix' patch later. Also I
realise this revision of binding hasn't been reviewed by Rob. Maybe I
should drop the patch for now.

> > +
> > +  arm,func-id:
> > +    description: |
> > +      An 32-bit value specifying the function ID used by the mailbox.
>
>          A single 32-bit value ...
>
> > +      The function ID follow the ARM SMC calling convention standard [1].
>
>                          follows
>
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +required:
> > +  - compatible
> > +  - "#mbox-cells"
> > +
> > +examples:
> > +  - |
> > +    sram@93f000 {
> > +      compatible = "mmio-sram";
> > +      reg = <0x0 0x93f000 0x0 0x1000>;
> > +      #address-cells = <1>;
> > +      #size-cells = <1>;
> > +      ranges = <0x0 0x93f000 0x1000>;
> > +
> > +      cpu_scp_lpri: scp-shmem@0 {
> > +        compatible = "arm,scmi-shmem";
> > +        reg = <0x0 0x200>;
> > +      };
> > +    };
> > +
> > +    smc_tx_mbox: tx_mbox {
> > +      #mbox-cells = <1>;
>
> As mentioned above, should be 0.
>
> > +      compatible = "arm,smc-mbox";
> > +      /* optional */
>
> First: having "optional" in a specific example is not helpful, just confusing.
> Second: It is actually *not* optional in this case, as there is no other way of propagating the function ID. The SCMI driver as the mailbox client has certainly no clue about this.
> I think I said this previously: Relying on the mailbox client to pass the function ID sounds broken, as this is a property of the mailbox controller driver. The mailbox client does not care about this mailbox communication detail, it just wants to trigger the mailbox.
>
Again, the mailbox controller here is the SMC/HVC _instruction_, which
doesn't care what value the first argument carry.

Cheers!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* RE: [PATCH V3 0/4] clk: imx8m: fix glitch/mux
From: Peng Fan @ 2019-09-18  5:45 UTC (permalink / raw)
  To: Stephen Boyd, festevam@gmail.com, mturquette@baylibre.com,
	s.hauer@pengutronix.de, shawnguo@kernel.org
  Cc: Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
	dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
	linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Jacky Bai
In-Reply-To: <20190917162820.8DC542067B@mail.kernel.org>

Hi Stephen,

> Subject: RE: [PATCH V3 0/4] clk: imx8m: fix glitch/mux
> 
> Quoting Peng Fan (2019-09-16 23:20:15)
> > Hi Stephen, Shawn,
> >
> > > Subject: [PATCH V3 0/4] clk: imx8m: fix glitch/mux
> >
> > Sorry to ping early. Is there a chance to land this patchset in 5.3 release?
> >
> 
> No, it won't be in 5.3 because that version is released. Shawn already sent the
> PR for 5.4 too so this will most likely be in v5.5 at the earliest.

Thanks for the info. But this patchset is bugfix, so hope this could be accepted in 5.4.

Thanks,
Peng.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [linux-sunxi] [PATCH] clk: sunxi-ng: h6: Use sigma-delta modulation for audio PLL
From: Chen-Yu Tsai @ 2019-09-18  5:46 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: Stephen Boyd, Mike Turquette, linux-kernel, Maxime Ripard,
	linux-sunxi, linux-clk, linux-arm-kernel
In-Reply-To: <8129141.yvSaxnLE4m@jernej-laptop>

On Wed, Sep 18, 2019 at 1:21 PM Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>
> Dne torek, 17. september 2019 ob 08:54:08 CEST je Chen-Yu Tsai napisal(a):
> > On Sat, Sep 14, 2019 at 9:51 PM Jernej Skrabec <jernej.skrabec@siol.net>
> wrote:
> > > Audio devices needs exact clock rates in order to correctly reproduce
> > > the sound. Until now, only integer factors were used to configure H6
> > > audio PLL which resulted in inexact rates. Fix that by adding support
> > > for fractional factors using sigma-delta modulation look-up table. It
> > > contains values for two most commonly used audio base frequencies.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > ---
> > >
> > >  drivers/clk/sunxi-ng/ccu-sun50i-h6.c | 21 +++++++++++++++------
> > >  1 file changed, 15 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> > > b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c index d89353a3cdec..ed6338d74474
> > > 100644
> > > --- a/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> > > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-h6.c
> > > @@ -203,12 +203,21 @@ static struct ccu_nkmp pll_hsic_clk = {
> > >
> > >   * hardcode it to match with the clock names.
> > >   */
> > >
> > >  #define SUN50I_H6_PLL_AUDIO_REG                0x078
> > >
> > > +
> > > +static struct ccu_sdm_setting pll_audio_sdm_table[] = {
> > > +       { .rate = 541900800, .pattern = 0xc001288d, .m = 1, .n = 22 },
> > > +       { .rate = 589824000, .pattern = 0xc00126e9, .m = 1, .n = 24 },
> > > +};
> > > +
> > >
> > >  static struct ccu_nm pll_audio_base_clk = {
> > >
> > >         .enable         = BIT(31),
> > >         .lock           = BIT(28),
> > >         .n              = _SUNXI_CCU_MULT_MIN(8, 8, 12),
> > >         .m              = _SUNXI_CCU_DIV(1, 1), /* input divider */
> > >
> > > +       .sdm            = _SUNXI_CCU_SDM(pll_audio_sdm_table,
> > > +                                        BIT(24), 0x178, BIT(31)),
> > >
> > >         .common         = {
> > >
> > > +               .features       = CCU_FEATURE_SIGMA_DELTA_MOD,
> > >
> > >                 .reg            = 0x078,
> > >                 .hw.init        = CLK_HW_INIT("pll-audio-base", "osc24M",
> > >
> > >                                               &ccu_nm_ops,
> > >
> > > @@ -753,12 +762,12 @@ static const struct clk_hw *clk_parent_pll_audio[] =
> > > {>
> > >  };
> > >
> > >  /*
> > >
> > > - * The divider of pll-audio is fixed to 8 now, as pll-audio-4x has a
> > > - * fixed post-divider 2.
> > > + * The divider of pll-audio is fixed to 24 for now, so 24576000 and
> > > 22579200 + * rates can be set exactly in conjunction with sigma-delta
> > > modulation.>
> > >   */
> > >
> > >  static CLK_FIXED_FACTOR_HWS(pll_audio_clk, "pll-audio",
> > >
> > >                             clk_parent_pll_audio,
> > >
> > > -                           8, 1, CLK_SET_RATE_PARENT);
> > > +                           24, 1, CLK_SET_RATE_PARENT);
> > >
> > >  static CLK_FIXED_FACTOR_HWS(pll_audio_2x_clk, "pll-audio-2x",
> > >
> > >                             clk_parent_pll_audio,
> > >                             4, 1, CLK_SET_RATE_PARENT);
> >
> > You need to fix the factors for the other two outputs as well, since all
> > three are derived from pll-audio-base.
>
> Fix how? pll-audio-2x and pll-audio-4x clocks have fixed divider in regards to
> pll-audio-base, while pll-audio has not. Unless you mean changing their name?

Argh... I got it wrong. It looks good actually.

Acked-by: Chen-Yu Tsai <wens@csie.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* RE: [PATCH V3 0/4] clk: imx8m: fix glitch/mux
From: Stephen Boyd @ 2019-09-18  5:53 UTC (permalink / raw)
  To: festevam@gmail.com, mturquette@baylibre.com,
	s.hauer@pengutronix.de, shawnguo@kernel.org, Peng Fan
  Cc: Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
	dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
	linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Jacky Bai
In-Reply-To: <AM0PR04MB4481D54C4508152E458BA9BE888E0@AM0PR04MB4481.eurprd04.prod.outlook.com>

Quoting Peng Fan (2019-09-17 22:45:20)
> Hi Stephen,
> 
> > Subject: RE: [PATCH V3 0/4] clk: imx8m: fix glitch/mux
> > 
> > Quoting Peng Fan (2019-09-16 23:20:15)
> > > Hi Stephen, Shawn,
> > >
> > > > Subject: [PATCH V3 0/4] clk: imx8m: fix glitch/mux
> > >
> > > Sorry to ping early. Is there a chance to land this patchset in 5.3 release?
> > >
> > 
> > No, it won't be in 5.3 because that version is released. Shawn already sent the
> > PR for 5.4 too so this will most likely be in v5.5 at the earliest.
> 
> Thanks for the info. But this patchset is bugfix, so hope this could be accepted in 5.4.
> 

Ok. Then let's throw it into 5.4 PR and see what goes wrong.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] ARM: zx: Use devm_platform_ioremap_resource() in zx296702_pd_probe()
From: Markus Elfring @ 2019-09-18  5:57 UTC (permalink / raw)
  To: linux-arm-kernel, Jun Nie, Shawn Guo, Russell King
  Cc: Bartosz Golaszewski, kernel-janitors, LKML, Himanshu Jha

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 18 Sep 2019 07:40:26 +0200

Simplify this function implementation by using a known wrapper function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/arm/mach-zx/zx296702-pm-domain.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/arch/arm/mach-zx/zx296702-pm-domain.c b/arch/arm/mach-zx/zx296702-pm-domain.c
index 7a08bf9dd792..ba4f556b7a13 100644
--- a/arch/arm/mach-zx/zx296702-pm-domain.c
+++ b/arch/arm/mach-zx/zx296702-pm-domain.c
@@ -152,7 +152,6 @@ static struct generic_pm_domain *zx296702_pm_domains[] = {
 static int zx296702_pd_probe(struct platform_device *pdev)
 {
 	struct genpd_onecell_data *genpd_data;
-	struct resource *res;
 	int i;

 	genpd_data = devm_kzalloc(&pdev->dev, sizeof(*genpd_data), GFP_KERNEL);
@@ -161,14 +160,7 @@ static int zx296702_pd_probe(struct platform_device *pdev)

 	genpd_data->domains = zx296702_pm_domains;
 	genpd_data->num_domains = ARRAY_SIZE(zx296702_pm_domains);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "no memory resource defined\n");
-		return -ENODEV;
-	}
-
-	pcubase = devm_ioremap_resource(&pdev->dev, res);
+	pcubase = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(pcubase)) {
 		dev_err(&pdev->dev, "ioremap fail.\n");
 		return -EIO;
--
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* RE: [PATCH 3/6] Timer: expose monotonic clock and counter value
From: Jianyong Wu (Arm Technology China) @ 2019-09-18  6:01 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Mark Rutland, Justin He (Arm Technology China), Suzuki Poulose,
	netdev@vger.kernel.org, Steve Capper, Will Deacon,
	linux-kernel@vger.kernel.org, sean.j.christopherson@intel.com,
	Kaly Xin (Arm Technology China), maz@kernel.org,
	pbonzini@redhat.com, nd, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190918034235.GA1469@localhost>

Hi Richard,

> -----Original Message-----
> From: Richard Cochran <richardcochran@gmail.com>
> Sent: Wednesday, September 18, 2019 11:43 AM
> To: Jianyong Wu (Arm Technology China) <Jianyong.Wu@arm.com>
> Cc: netdev@vger.kernel.org; pbonzini@redhat.com;
> sean.j.christopherson@intel.com; maz@kernel.org; Mark Rutland
> <Mark.Rutland@arm.com>; Will Deacon <Will.Deacon@arm.com>; Suzuki
> Poulose <Suzuki.Poulose@arm.com>; linux-kernel@vger.kernel.org; Steve
> Capper <Steve.Capper@arm.com>; Kaly Xin (Arm Technology China)
> <Kaly.Xin@arm.com>; Justin He (Arm Technology China)
> <Justin.He@arm.com>; nd <nd@arm.com>; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH 3/6] Timer: expose monotonic clock and counter value
> 
> On Tue, Sep 17, 2019 at 07:24:27AM -0400, Jianyong Wu wrote:
> > A number of PTP drivers (such as ptp-kvm) are assuming what the
> > current clock source is, which could lead to interesting effects on
> > systems where the clocksource can change depending on external events.
> >
> > For this purpose, add a new API that retrives both the current
> > monotonic clock as well as its counter value.
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
> > ---
> >  include/linux/timekeeping.h |  3 +++
> >  kernel/time/timekeeping.c   | 13 +++++++++++++
> >  2 files changed, 16 insertions(+)
> 
> For core time keeping changes, you must CC lkml, tglx, and John Stultz.
> 

Thanks, I will.

Thanks
Jianyong Wu

> Thanks,
> Richard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V3 1/4] clk: imx: pll14xx: avoid glitch when set rate
From: Stephen Boyd @ 2019-09-18  6:07 UTC (permalink / raw)
  To: festevam@gmail.com, mturquette@baylibre.com,
	s.hauer@pengutronix.de, shawnguo@kernel.org, Peng Fan
  Cc: Peng Fan, Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
	dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
	linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Jacky Bai
In-Reply-To: <1568043491-20680-2-git-send-email-peng.fan@nxp.com>

Quoting Peng Fan (2019-09-08 20:39:34)
> From: Peng Fan <peng.fan@nxp.com>
> 
> According to PLL1443XA and PLL1416X spec,
> "When BYPASS is 0 and RESETB is changed from 0 to 1, FOUT starts to
> output unstable clock until lock time passes. PLL1416X/PLL1443XA may
> generate a glitch at FOUT."
> 
> So set BYPASS when RESETB is changed from 0 to 1 to avoid glitch.
> In the end of set rate, BYPASS will be cleared.
> 
> When prepare clock, also need to take care to avoid glitch. So
> we also follow Spec to set BYPASS before RESETB changed from 0 to 1.
> And add a check if the RESETB is already 0, directly return 0;
> 
> Fixes: 8646d4dcc7fb ("clk: imx: Add PLLs driver for imx8mm soc")
> Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

Applied to clk-next


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V3 2/4] clk: imx: clk-pll14xx: unbypass PLL by default
From: Stephen Boyd @ 2019-09-18  6:07 UTC (permalink / raw)
  To: festevam@gmail.com, mturquette@baylibre.com,
	s.hauer@pengutronix.de, shawnguo@kernel.org, Peng Fan
  Cc: Peng Fan, Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
	dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
	linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Jacky Bai
In-Reply-To: <1568043491-20680-3-git-send-email-peng.fan@nxp.com>

Quoting Peng Fan (2019-09-08 20:39:39)
> From: Peng Fan <peng.fan@nxp.com>
> 
> When registering the PLL, unbypass the PLL.
> The PLL has two bypass control bit, BYPASS and EXT_BYPASS.
> we will expose EXT_BYPASS to clk driver for mux usage, and keep
> BYPASS inside pll14xx usage. The PLL has a restriction that
> when M/P change, need to RESET/BYPASS pll to avoid glitch, so
> we could not expose BYPASS.
> 
> To make it easy for clk driver usage, unbypass PLL which does
> not hurt current function.
> 
> Fixes: 8646d4dcc7fb ("clk: imx: Add PLLs driver for imx8mm soc")
> Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

Applied to clk-next


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V3 3/4] clk: imx: imx8mm: fix pll mux bit
From: Stephen Boyd @ 2019-09-18  6:07 UTC (permalink / raw)
  To: festevam@gmail.com, mturquette@baylibre.com,
	s.hauer@pengutronix.de, shawnguo@kernel.org, Peng Fan
  Cc: Peng Fan, Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
	dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
	linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Jacky Bai
In-Reply-To: <1568043491-20680-4-git-send-email-peng.fan@nxp.com>

Quoting Peng Fan (2019-09-08 20:39:44)
> From: Peng Fan <peng.fan@nxp.com>
> 
> pll BYPASS bit should be kept inside pll driver for glitchless freq
> setting following spec. If exposing the bit, that means pll driver and
> clk driver has two paths to touch this bit, which is wrong.
> 
> So use EXT_BYPASS bit here.
> 
> And drop uneeded set parent, because EXT_BYPASS default is 0.
> 
> Fixes: ba5625c3e272 ("clk: imx: Add clock driver support for imx8mm")
> Suggested-by: Jacky Bai <ping.bai@nxp.com>
> Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

Applied to clk-next


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V3 4/4] clk: imx: imx8mn: fix pll mux bit
From: Stephen Boyd @ 2019-09-18  6:07 UTC (permalink / raw)
  To: festevam@gmail.com, mturquette@baylibre.com,
	s.hauer@pengutronix.de, shawnguo@kernel.org, Peng Fan
  Cc: Peng Fan, Abel Vesa, Anson Huang, linux-kernel@vger.kernel.org,
	dl-linux-imx, kernel@pengutronix.de, Leonard Crestez,
	linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Jacky Bai
In-Reply-To: <1568043491-20680-5-git-send-email-peng.fan@nxp.com>

Quoting Peng Fan (2019-09-08 20:39:50)
> From: Peng Fan <peng.fan@nxp.com>
> 
> pll BYPASS bit should be kept inside pll driver for glitchless freq
> setting following spec. If exposing the bit, that means pll driver and
> clk driver has two paths to touch this bit, which is wrong.
> 
> So use EXT_BYPASS bit here.
> 
> And drop uneeded set parent, because EXT_BYPASS default is 0.
> 
> Suggested-by: Jacky Bai <ping.bai@nxp.com>
> Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

Applied to clk-next


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH V5 00/11] clk: imx8: add new clock binding for better pm support
From: Stephen Boyd @ 2019-09-18  6:08 UTC (permalink / raw)
  To: Dong Aisheng, linux-clk
  Cc: Dong Aisheng, mturquette, linux-imx, kernel, fabio.estevam,
	shawnguo, linux-arm-kernel
In-Reply-To: <1568081408-26800-1-git-send-email-aisheng.dong@nxp.com>

Quoting Dong Aisheng (2019-09-09 19:09:57)
> This is a follow up of this patch series.
> https://patchwork.kernel.org/cover/10924029/
> [V2,0/2] clk: imx: scu: add parsing clocks from device tree support
> 
> This patch series is a preparation for the MX8 Architecture improvement.
> As for IMX SCU based platforms like MX8QM and MX8QXP, they are comprised
> of a couple of SS(Subsystems) while most of them within the same SS
> can be shared. e.g. Clocks, Devices and etc.
> 
> However, current clock binding is using SW IDs for device tree to use
> which can cause troubles in writing the common <soc>-ss-xx.dtsi file for
> different SoCs.
> 
> This patch series aims to introduce a new binding which is more close to
> hardware and platform independent and can makes us write a more general
> drivers for different SCU based SoCs.
> 
> Another important thing is that on MX8, each Clock resource is associated
> with a power domain. So we have to attach that clock device to the power
> domain in order to make it work properly. Further more, the clock state
> will be lost when its power domain is completely off during suspend/resume,
> so we also introduce the clock state save&restore mechanism.

I had some more comments on v4. I'm going to wait for those to be
addressed before reviewing this series.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 2/3] pinctrl: meson-a1: add pinctrl driver for Meson A1 Soc
From: Qianggui Song @ 2019-09-18  6:36 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Mark Rutland, Hanjie Lin, Jianxin Pan, Neil Armstrong,
	Martin Blumenstingl, Kevin Hilman, Linus Walleij, linux-kernel,
	linux-gpio, Rob Herring, linux-arm-kernel, Carlo Caione,
	linux-amlogic, Xingyu Chen
In-Reply-To: <1j8sqn3tjt.fsf@starbuckisacylon.baylibre.com>


On 2019/9/17 22:07, Jerome Brunet wrote:
> 
> On Tue 17 Sep 2019 at 13:51, Qianggui Song <qianggui.song@amlogic.com> wrote:
>>>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
>>>> index 8bba9d0..885b89d 100644
>>>> --- a/drivers/pinctrl/meson/pinctrl-meson.c
>>>> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
>>>> @@ -688,8 +688,12 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
>>>>  
>>>>  	pc->reg_ds = meson_map_resource(pc, gpio_np, "ds");
>>>>  	if (IS_ERR(pc->reg_ds)) {
>>>> -		dev_dbg(pc->dev, "ds registers not found - skipping\n");
>>>> -		pc->reg_ds = NULL;
>>>> +		if (pc->data->reg_layout == A1_LAYOUT) {
>>>> +			pc->reg_ds = pc->reg_pullen;
>>>
>>> IMO, this kind of ID based init fixup is not going to scale and will
>>> lead to something difficult to maintain in the end.
>>>
>>> The way the different register sets interract with each other is already
>>> pretty complex to follow.
>>>
>>> You could rework this in 2 different ways:
>>> #1 - Have the generic function parse all the register sets and have all
>>> drivers provide a specific (as in gxbb, gxl, axg, etc ...)  function to :
>>>  - Verify the expected sets have been provided
>>>  - Make assignement fixup as above if necessary
>>>
>>> #2 - Rework the driver to have only one single register region
>>>  I think one of your colleague previously mentionned this was not
>>>  possible. It is still unclear to me why ...
>>>
>> Appreciate your advice.  I have an idea based on #1, how about providing
>> only two dt parse function, one is for chips before A1(the old one),
>> another is for A1 and later chips that share the same layout. Assign
>> these two functions to their own driver.
> 
> That's roughly the same thing as your initial proposition with function
> pointer instead of IDs ... IMO, this would still be a quick fix to
> address your immediate topic instead of dealing with the driver as
> whole, which is my concern here.
> 
For #1. It would be like
generic_parse_dt()
{
	1. parse all register regions (mux gpio pull pull_en ds)
	
	2. call  specific function through function pointer in
 	   meson_pinctrl_data.(each platform should have AO and EE two
           specific functions for they are not the same)
	{
		do work you mentioned above
	}
}
right ?
If that so, maybe there are a lot of duplicated codes for most Socs
share the same reg layout. So I guess five specific functions are
enough: AXG and before(ao,ee), G12A(ao,ee) and A1(will place them in
pinctrl_meson.c). Since m8 to AXG are the same register layout for both
ee and ao, G12A with new feature ds and new ao register layout.

Or I misunderstood the #1 ?
>>>> +		} else {
>>>> +			dev_dbg(pc->dev, "ds registers not found - skipping\n");
>>>> +			pc->reg_ds = NULL;
>>>> +		}
>>>>  	}
>>>>  
>>>>  	return 0;
>>>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
>>>> index c696f32..3d0c58d 100644
>>>> --- a/drivers/pinctrl/meson/pinctrl-meson.h
>>>> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
>>>> @@ -80,6 +80,14 @@ enum meson_pinconf_drv {
>>>>  };
>>>>  
>>>>  /**
>>>> + * enum meson_reg_layout - identify two types of reg layout
>>>> + */
>>>> +enum meson_reg_layout {
>>>> +	LEGACY_LAYOUT,
>>>> +	A1_LAYOUT,
>>>> +};
>>>> +
>>>> +/**
>>>>   * struct meson bank
>>>>   *
>>>>   * @name:	bank name
>>>> @@ -114,6 +122,7 @@ struct meson_pinctrl_data {
>>>>  	unsigned int num_banks;
>>>>  	const struct pinmux_ops *pmx_ops;
>>>>  	void *pmx_data;
>>>> +	unsigned int reg_layout;
>>>>  };
>>>>  
>>>>  struct meson_pinctrl {
>>>
>>> .
>>>
> 
> .
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] of: restore old handling of cells_name=NULL in of_*_phandle_with_args()
From: Uwe Kleine-König @ 2019-09-18  6:38 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: devicetree, Geert Uytterhoeven, Uwe Kleine-König,
	Robin Murphy, Joerg Roedel, linux-kernel, linux-mediatek, kernel,
	Matthias Brugger, Geert Uytterhoeven, Will Deacon, Peter Rosin,
	linux-arm-kernel, Marek Szyprowski

From: Uwe Kleine-König <uwe@kleine-koenig.org>

Before commit e42ee61017f5 ("of: Let of_for_each_phandle fallback to
non-negative cell_count") the iterator functions calling
of_for_each_phandle assumed a cell count of 0 if cells_name was NULL.
This corner case was missed when implementing the fallback logic in
e42ee61017f5 and resulted in an endless loop.

Restore the old behaviour of of_count_phandle_with_args() and
of_parse_phandle_with_args() and add a check to
of_phandle_iterator_init() to prevent a similar failure as a safety
precaution. of_parse_phandle_with_args_map() doesn't need a similar fix
as cells_name isn't NULL there.

Affected drivers are:
 - drivers/base/power/domain.c
 - drivers/base/power/domain.c
 - drivers/clk/ti/clk-dra7-atl.c
 - drivers/hwmon/ibmpowernv.c
 - drivers/i2c/muxes/i2c-demux-pinctrl.c
 - drivers/iommu/mtk_iommu.c
 - drivers/net/ethernet/freescale/fman/mac.c
 - drivers/opp/of.c
 - drivers/perf/arm_dsu_pmu.c
 - drivers/regulator/of_regulator.c
 - drivers/remoteproc/imx_rproc.c
 - drivers/soc/rockchip/pm_domains.c
 - sound/soc/fsl/imx-audmix.c
 - sound/soc/fsl/imx-audmix.c
 - sound/soc/meson/axg-card.c
 - sound/soc/samsung/tm2_wm5110.c
 - sound/soc/samsung/tm2_wm5110.c

Thanks to Geert Uytterhoeven for reporting the issue, Peter Rosin for
helping pinpoint the actual problem and the testers for confirming this
fix.

Fixes: e42ee61017f5 ("of: Let of_for_each_phandle fallback to non-negative cell_count")
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

compared to the untested patch I sent yesterday I also fixed
of_parse_phandle_with_args which has three users that pass
cells_name=NULL. (i.e. drivers/clk/ti/clk-dra7-atl.c,
sound/soc/fsl/imx-audmix.c, sound/soc/samsung/tm2_wm5110.c) I didn't
look closely, but maybe these could be converted to use of_parse_phandle
as there are no arguments to be processed with no cells_name?!

Best regards
Uwe

 drivers/of/base.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 2f25d2dfecfa..25ee07c0a3cd 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1286,6 +1286,13 @@ int of_phandle_iterator_init(struct of_phandle_iterator *it,
 
 	memset(it, 0, sizeof(*it));
 
+	/*
+	 * one of cell_count or cells_name must be provided to determine the
+	 * argument length.
+	 */
+	if (cell_count < 0 && !cells_name)
+		return -EINVAL;
+
 	list = of_get_property(np, list_name, &size);
 	if (!list)
 		return -ENOENT;
@@ -1512,10 +1519,17 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 				const char *cells_name, int index,
 				struct of_phandle_args *out_args)
 {
+	int cell_count = -1;
+
 	if (index < 0)
 		return -EINVAL;
-	return __of_parse_phandle_with_args(np, list_name, cells_name, -1,
-					    index, out_args);
+
+	/* If cells_name if NULL we assume a cell count of 0 */
+	if (!cells_name)
+		cell_count = 0;
+
+	return __of_parse_phandle_with_args(np, list_name, cells_name,
+					    cell_count, index, out_args);
 }
 EXPORT_SYMBOL(of_parse_phandle_with_args);
 
@@ -1765,6 +1779,18 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
 	struct of_phandle_iterator it;
 	int rc, cur_index = 0;
 
+	/* If cells_name is NULL we assume a cell count of 0 */
+	if (cells_name == NULL) {
+		const __be32 *list;
+		int size;
+
+		list = of_get_property(np, list_name, &size);
+		if (!list)
+			return -ENOENT;
+
+		return size / sizeof(*list);
+	}
+
 	rc = of_phandle_iterator_init(&it, np, list_name, cells_name, -1);
 	if (rc)
 		return rc;
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] irqchip/gic-v3: Fix GIC_LINE_NR accessor
From: Zenghui Yu @ 2019-09-18  6:57 UTC (permalink / raw)
  To: maz
  Cc: jason, linux-kernel, Zenghui Yu, wanghaibin.wang, tglx,
	linux-arm-kernel

As per GIC spec, ITLinesNumber indicates the maximum SPI INTID that
the GIC implementation supports. And the maximum SPI INTID an
implementation might support is 1019 (field value 11111).

max(GICD_TYPER_SPIS(...), 1020) is not what we actually want for
GIC_LINE_NR. Fix it to min(GICD_TYPER_SPIS(...), 1020).

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---

Hi Marc,

I still see "GICv3: 992 SPIs implemented" on the host. I go back to
https://patchwork.kernel.org/patch/11078623/ and it seems that we
failed to make the GIC_LINE_NR correct at that time.

 drivers/irqchip/irq-gic-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 7b0c96b9e02f..f4a49aef5ca4 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -59,7 +59,7 @@ static struct gic_chip_data gic_data __read_mostly;
 static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key);
 
 #define GIC_ID_NR	(1U << GICD_TYPER_ID_BITS(gic_data.rdists.gicd_typer))
-#define GIC_LINE_NR	max(GICD_TYPER_SPIS(gic_data.rdists.gicd_typer), 1020U)
+#define GIC_LINE_NR	min(GICD_TYPER_SPIS(gic_data.rdists.gicd_typer), 1020U)
 #define GIC_ESPI_NR	GICD_TYPER_ESPIS(gic_data.rdists.gicd_typer)
 
 /*
-- 
2.19.1



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH] hwrng: iproc-rng200 - Use devm_platform_ioremap_resource() in iproc_rng200_probe()
From: Markus Elfring @ 2019-09-18  7:19 UTC (permalink / raw)
  To: linux-crypto, linux-arm-kernel, bcm-kernel-feedback-list,
	Arnd Bergmann, Florian Fainelli, Greg Kroah-Hartman, Herbert Xu,
	Matt Mackall, Ray Jui, Scott Branden
  Cc: Bartosz Golaszewski, kernel-janitors, LKML, Himanshu Jha

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 18 Sep 2019 09:09:22 +0200

Simplify this function implementation by using a known wrapper function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/char/hw_random/iproc-rng200.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/char/hw_random/iproc-rng200.c b/drivers/char/hw_random/iproc-rng200.c
index 92be1c0ab99f..899ff25f4f28 100644
--- a/drivers/char/hw_random/iproc-rng200.c
+++ b/drivers/char/hw_random/iproc-rng200.c
@@ -181,7 +181,6 @@ static void iproc_rng200_cleanup(struct hwrng *rng)
 static int iproc_rng200_probe(struct platform_device *pdev)
 {
 	struct iproc_rng200_dev *priv;
-	struct resource *res;
 	struct device *dev = &pdev->dev;
 	int ret;

@@ -190,13 +189,7 @@ static int iproc_rng200_probe(struct platform_device *pdev)
 		return -ENOMEM;

 	/* Map peripheral */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(dev, "failed to get rng resources\n");
-		return -EINVAL;
-	}
-
-	priv->base = devm_ioremap_resource(dev, res);
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->base)) {
 		dev_err(dev, "failed to remap rng regs\n");
 		return PTR_ERR(priv->base);
--
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v5 0/5] Add HDMI jack support on RK3288
From: Cheng-yi Chiang @ 2019-09-18  7:28 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: ALSA development, Heiko Stuebner, David Airlie, dri-devel,
	Liam Girdwood, Hans Verkuil, Andrzej Hajda, Laurent Pinchart,
	linux-rockchip, Takashi Iwai, Tzung-Bi Shih, Dylan Reid, tzungbi,
	Jonas Karlman, Russell King, Mark Brown, Jaroslav Kysela,
	linux-arm-kernel, Jernej Skrabec, Douglas Anderson,
	Linux Kernel Mailing List, Daniel Vetter
In-Reply-To: <3fc94731-f66a-223d-995e-97ac67f9e882@baylibre.com>

On Fri, Sep 13, 2019 at 5:33 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Hi,
>
> On 09/09/2019 15:53, Mark Brown wrote:
> > On Mon, Sep 09, 2019 at 09:37:14AM +0200, Neil Armstrong wrote:
> >
> >> I'd like some review from ASoC people and other drm bridge reviewers,
> >> Jernej, Jonas & Andrzej.
> >
> >> Jonas could have some comments on the overall patchset.
> >
> > The ASoC bits look basically fine, I've gone ahead and applied
> > patch 1 as is since we're just before the merge window and that
> > way we reduce potential cross tree issues.  I know there's a lot
> > of discussion on the DRM side about how they want to handle
> > things with jacks, I'm not 100% sure what the latest thinking is
> > there.
> >
>
> Thanks Mark.
>
>
> Cheng-Yi can you resent this serie without the already applied
> first patch with Jernej, Jonas, and Jerome Brunet <jbrunet@baylibre.com> CCed ?

Hi Neil,
Got it. Sorry for the late reply.
I will resend this series without the first patch, based on latest
drm-misc-next, and cc the folks.
Thanks!

>
> Thanks,
> Neil
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] mm/pgtable/debug: Fix test validating architecture page table helpers
From: Anshuman Khandual @ 2019-09-18  7:32 UTC (permalink / raw)
  To: Christophe Leroy, linux-mm
  Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
	Heiko Carstens, Michal Hocko, Dave Hansen, Paul Mackerras,
	sparclinux, Dan Williams, linux-s390, Jason Gunthorpe, x86,
	Russell King - ARM Linux, Matthew Wilcox, Steven Price,
	Tetsuo Handa, Vlastimil Babka, linux-snps-arc, Kees Cook,
	Mark Brown, Kirill A . Shutemov, Thomas Gleixner, Gerald Schaefer,
	linux-arm-kernel, Sri Krishna chowdary, Masahiro Yamada,
	Greg Kroah-Hartman, Ard Biesheuvel, linux-mips, Ralf Baechle,
	linux-kernel, Paul Burton, Mike Rapoport, Vineet Gupta,
	Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <1892b37d1fd9a4ed39e76c4b999b6556077201c0.1568355752.git.christophe.leroy@c-s.fr>



On 09/13/2019 11:53 AM, Christophe Leroy wrote:
> Fix build failure on powerpc.
> 
> Fix preemption imbalance.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  mm/arch_pgtable_test.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c
> index 8b4a92756ad8..f2b3c9ec35fa 100644
> --- a/mm/arch_pgtable_test.c
> +++ b/mm/arch_pgtable_test.c
> @@ -24,6 +24,7 @@
>  #include <linux/swap.h>
>  #include <linux/swapops.h>
>  #include <linux/sched/mm.h>
> +#include <linux/highmem.h>
>  #include <asm/pgalloc.h>
>  #include <asm/pgtable.h>
>  
> @@ -400,6 +401,8 @@ static int __init arch_pgtable_tests_init(void)
>  	p4d_clear_tests(p4dp);
>  	pgd_clear_tests(mm, pgdp);
>  
> +	pte_unmap(ptep);
> +
>  	pmd_populate_tests(mm, pmdp, saved_ptep);
>  	pud_populate_tests(mm, pudp, saved_pmdp);
>  	p4d_populate_tests(mm, p4dp, saved_pudp);
> 

Hello Christophe,

I am planning to fold this fix into the current patch and retain your
Signed-off-by. Are you okay with it ?

- Anshuman

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5] gpio/mpc8xxx: change irq handler from chained to normal
From: Bartosz Golaszewski @ 2019-09-18  7:36 UTC (permalink / raw)
  To: Hui Song
  Cc: Mark Rutland, linux-devicetree, Linus Walleij, LKML, Li Yang,
	linux-gpio, Rob Herring, Shawn Guo, arm-soc
In-Reply-To: <20190916055817.43425-1-hui.song_1@nxp.com>

pon., 16 wrz 2019 o 08:08 Hui Song <hui.song_1@nxp.com> napisał(a):
>
> From: Song Hui <hui.song_1@nxp.com>
>
> More than one gpio controllers can share one interrupt, change the
> driver to request shared irq.
>
> Signed-off-by: Laurentiu Tudor <Laurentiu.Tudor@nxp.com>
> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
> Signed-off-by: Song Hui <hui.song_1@nxp.com>
> ---
> Changes in v5:
>         - add traverse every bit function.
> Changes in v4:
>         - convert 'pr_err' to 'dev_err'.
> Changes in v3:
>         - update the patch description.
> Changes in v2:
>         - delete the compatible of ls1088a.
>  drivers/gpio/gpio-mpc8xxx.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> index 16a47de..3a06ca9 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -22,6 +22,7 @@
>  #include <linux/irq.h>
>  #include <linux/gpio/driver.h>
>  #include <linux/bitops.h>
> +#include <linux/interrupt.h>
>
>  #define MPC8XXX_GPIO_PINS      32
>
> @@ -127,20 +128,20 @@ static int mpc8xxx_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
>                 return -ENXIO;
>  }
>
> -static void mpc8xxx_gpio_irq_cascade(struct irq_desc *desc)
> +static irqreturn_t mpc8xxx_gpio_irq_cascade(int irq, void *data)
>  {
> -       struct mpc8xxx_gpio_chip *mpc8xxx_gc = irq_desc_get_handler_data(desc);
> -       struct irq_chip *chip = irq_desc_get_chip(desc);
> +       struct mpc8xxx_gpio_chip *mpc8xxx_gc = (struct mpc8xxx_gpio_chip *)data;

There's no need to cast void pointers.

>         struct gpio_chip *gc = &mpc8xxx_gc->gc;
>         unsigned int mask;
> +       int i;
>
>         mask = gc->read_reg(mpc8xxx_gc->regs + GPIO_IER)
>                 & gc->read_reg(mpc8xxx_gc->regs + GPIO_IMR);
> -       if (mask)
> +       for_each_set_bit(i, &mask, 32)
>                 generic_handle_irq(irq_linear_revmap(mpc8xxx_gc->irq,
> -                                                    32 - ffs(mask)));
> -       if (chip->irq_eoi)
> -               chip->irq_eoi(&desc->irq_data);
> +                                                    31 - i));
> +
> +       return IRQ_HANDLED;
>  }
>
>  static void mpc8xxx_irq_unmask(struct irq_data *d)
> @@ -388,8 +389,8 @@ static int mpc8xxx_probe(struct platform_device *pdev)
>
>         ret = gpiochip_add_data(gc, mpc8xxx_gc);
>         if (ret) {
> -               pr_err("%pOF: GPIO chip registration failed with status %d\n",
> -                      np, ret);
> +               dev_err(&pdev->dev, "%pOF: GPIO chip registration failed with status %d\n",
> +                       np, ret);
>                 goto err;
>         }
>
> @@ -409,8 +410,15 @@ static int mpc8xxx_probe(struct platform_device *pdev)
>         if (devtype->gpio_dir_in_init)
>                 devtype->gpio_dir_in_init(gc);
>
> -       irq_set_chained_handler_and_data(mpc8xxx_gc->irqn,
> -                                        mpc8xxx_gpio_irq_cascade, mpc8xxx_gc);
> +       ret = request_irq(mpc8xxx_gc->irqn, mpc8xxx_gpio_irq_cascade,
> +                         IRQF_NO_THREAD | IRQF_SHARED, "gpio-cascade",
> +                         mpc8xxx_gc);

You never free this irq. Maybe use devm_request_irq()?

Bart

> +       if (ret) {
> +               dev_err(&pdev->dev, "%s: failed to request_irq(%d), ret = %d\n",
> +                       np->full_name, mpc8xxx_gc->irqn, ret);
> +               goto err;
> +       }
> +
>         return 0;
>  err:
>         iounmap(mpc8xxx_gc->regs);
> --
> 2.9.5
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] hwrng: mediatek: Use devm_platform_ioremap_resource() in mtk_rng_probe()
From: Markus Elfring @ 2019-09-18  7:44 UTC (permalink / raw)
  To: linux-crypto, linux-arm-kernel, linux-mediatek, Arnd Bergmann,
	Greg Kroah-Hartman, Herbert Xu, Matt Mackall, Matthias Brugger,
	Sean Wang
  Cc: Bartosz Golaszewski, kernel-janitors, LKML, Himanshu Jha

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 18 Sep 2019 09:34:11 +0200

Simplify this function implementation by using a known wrapper function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/char/hw_random/mtk-rng.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/char/hw_random/mtk-rng.c b/drivers/char/hw_random/mtk-rng.c
index e649be5a5f13..8ad7b515a51b 100644
--- a/drivers/char/hw_random/mtk-rng.c
+++ b/drivers/char/hw_random/mtk-rng.c
@@ -105,16 +105,9 @@ static int mtk_rng_read(struct hwrng *rng, void *buf, size_t max, bool wait)

 static int mtk_rng_probe(struct platform_device *pdev)
 {
-	struct resource *res;
 	int ret;
 	struct mtk_rng *priv;

-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "no iomem resource\n");
-		return -ENXIO;
-	}
-
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
@@ -135,7 +128,7 @@ static int mtk_rng_probe(struct platform_device *pdev)
 		return ret;
 	}

-	priv->base = devm_ioremap_resource(&pdev->dev, res);
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);

--
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 0/7] counter: Simplify count_read/count_write/signal_read
From: William Breathitt Gray @ 2019-09-18  7:52 UTC (permalink / raw)
  To: jic23
  Cc: alexandre.torgue, linux-iio, patrick.havelange, linux-kernel,
	William Breathitt Gray, mcoquelin.stm32, fabrice.gasnier,
	linux-stm32, linux-arm-kernel

Changes in v2:
 - Update the rest of the drivers under drivers/counter

The changes in this patchset will not affect the userspace interface.
Rather, these changes are intended to simplify the kernelspace Counter
callbacks for counter device driver authors.

The following main changes are proposed:

* Retire the opaque counter_count_read_value/counter_count_write_value
  structures and simply represent count data as an unsigned integer.

* Retire the opaque counter_signal_read_value structure and represent
  Signal data as a counter_signal_value enum.

These changes should reduce some complexity and code in the use and
implementation of the count_read, count_write, and signal_read
callbacks.

The opaque structures for Count data and Signal data were introduced
originally in anticipation of supporting various representations of
counter data (e.g. arbitrary-precision tallies, floating-point spherical
coordinate positions, etc). However, with the counter device drivers
that have appeared, it's become apparent that utilizing opaque
structures in kernelspace is not the best approach to take.

I believe it is best to let userspace applications decide how to
interpret the count data they receive. There are a couple of reasons why
it would be good to do so:

* Users use their devices in unexpected ways.

  For example, a quadrature encoder counter device is typically used to
  keep track of the position of a motor, but a user could set the device
  in a pulse-direction mode and instead use it to count sporadic rising
  edges from an arbitrary signal line unrelated to positioning. Users
  should have the freedom to decide what their data represents.

* Most counter devices represent data as unsigned integers anyway.

  For example, whether the device is a tally counter or position
  counter, the count data is represented to the user as an unsigned
  integer value. So specifying that one device is representing tallies
  while the other specifies positions does not provide much utility from
  an interface perspective.

For these reasons, the count_read and count_write callbacks have been
redefined to pass count data directly as unsigned long instead of passed
via opaque structures:

        count_read(struct counter_device *counter,
                   struct counter_count *count, unsigned long *val);
        count_write(struct counter_device *counter,
                    struct counter_count *count, unsigned long val);

Similarly, the signal_read is redefined to pass Signal data directly as
a counter_signal_value enum instead of via an opaque structure:

        signal_read(struct counter_device *counter,
                    struct counter_signal *signal,
                    enum counter_signal_value *val);

The counter_signal_value enum is simply the counter_signal_level enum
redefined to remove the references to the Signal data "level" data type.

William Breathitt Gray (7):
  counter: Simplify the count_read and count_write callbacks
  counter: Simplify the signal_read callback
  docs: driver-api: generic-counter: Update Count and Signal data types
  counter: 104-quad-8: Update count_read/count_write/signal_read
    callbacks
  counter: ftm-quaddec: Update count_read and count_write callbacks
  counter: stm32-lptimer-cnt: Update count_read callback
  counter: stm32-timer-cnt: Update count_read and count_write callbacks

 Documentation/driver-api/generic-counter.rst |  22 ++--
 drivers/counter/104-quad-8.c                 |  33 ++----
 drivers/counter/counter.c                    | 101 +++----------------
 drivers/counter/ftm-quaddec.c                |  14 +--
 drivers/counter/stm32-lptimer-cnt.c          |   5 +-
 drivers/counter/stm32-timer-cnt.c            |  17 +---
 include/linux/counter.h                      |  74 ++------------
 7 files changed, 53 insertions(+), 213 deletions(-)

-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox