Quoting Álvaro Fernández Rojas (2023-03-21 13:10:22) > diff --git a/drivers/clk/bcm/clk-bcm63268-timer.c b/drivers/clk/bcm/clk-bcm63268-timer.c > new file mode 100644 > index 000000000000..6a1fdd193cb5 > --- /dev/null > +++ b/drivers/clk/bcm/clk-bcm63268-timer.c > @@ -0,0 +1,232 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * BCM63268 Timer Clock and Reset Controller Driver [...] > + > +static inline struct bcm63268_tclkrst_hw * > +to_bcm63268_timer_reset(struct reset_controller_dev *rcdev) > +{ > + return container_of(rcdev, struct bcm63268_tclkrst_hw, rcdev); > +} > + > +static int bcm63268_timer_reset_update(struct reset_controller_dev *rcdev, > + unsigned long id, bool assert) > +{ > + struct bcm63268_tclkrst_hw *reset = to_bcm63268_timer_reset(rcdev); > + unsigned long flags; > + uint32_t val; > + > + spin_lock_irqsave(&reset->lock, flags); > + val = __raw_readl(reset->regs); Use regular ol readl() here, unless you have some need for no barrires or byte swapping. > + if (assert) > + val &= ~BIT(id); > + else > + val |= BIT(id); > + __raw_writel(val, reset->regs); Same. > + spin_unlock_irqrestore(&reset->lock, flags); > + > + return 0; > +} > + [...] > + > +static int bcm63268_tclk_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + const struct bcm63268_tclk_table_entry *entry, *table; > + struct bcm63268_tclkrst_hw *hw; > + struct clk_hw *clk; > + u8 maxbit = 0; > + int i, ret; > + > + table = of_device_get_match_data(dev); Use device_get_match_data() instead. > + if (!table) > + return -EINVAL; > + > + for (entry = table; entry->name; entry++) > + maxbit = max(maxbit, entry->bit); > + maxbit++; > + > + hw = devm_kzalloc(&pdev->dev, struct_size(hw, data.hws, maxbit), > + GFP_KERNEL); > + if (!hw) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, hw); > + > + spin_lock_init(&hw->lock); > + > + hw->data.num = maxbit; > + for (i = 0; i < maxbit; i++) > + hw->data.hws[i] = ERR_PTR(-ENODEV); > + > + hw->regs = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(hw->regs)) > + return PTR_ERR(hw->regs); > + > + for (entry = table; entry->name; entry++) { > + clk = clk_hw_register_gate(dev, entry->name, NULL, 0, Use devm? > + hw->regs, entry->bit, > + CLK_GATE_BIG_ENDIAN, &hw->lock); > + if (IS_ERR(clk)) { > + ret = PTR_ERR(clk); > + goto out_err; > + } > + > + hw->data.hws[entry->bit] = clk; > + } > + > + ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, > + &hw->data); > + if (ret) > + return ret; > + > + hw->rcdev.of_node = dev->of_node; > + hw->rcdev.ops = &bcm63268_timer_reset_ops; > + > + ret = devm_reset_controller_register(dev, &hw->rcdev); > + if (ret) > + dev_err(dev, "Failed to register reset controller\n"); > + > + return 0; > + > +out_err: > + for (i = 0; i < hw->data.num; i++) { > + if (!IS_ERR(hw->data.hws[i])) > + clk_hw_unregister_gate(hw->data.hws[i]); And then drop this? > + } > + > + return ret; > +} > + > +static const struct of_device_id bcm63268_tclk_dt_ids[] = { > + { > + .compatible = "brcm,bcm63268-timer-clocks", > + .data = &bcm63268_timer_clocks, Are you planning on adding more SoCs to this driver? The data can currently be always assumed to be bcm63268_timer_clocks > + }, { > + /* sentinel */ > + } > +}; > +