Quoting Ryan Chen (2024-08-29 00:09:12)
> > Subject: Re: [PATCH v2 3/3] clk: aspeed: add AST2700 clk driver
> >
> > Quoting Ryan Chen (2024-08-27 23:27:40)
> > > a/drivers/clk/clk-ast2700.c b/drivers/clk/clk-ast2700.c new file mode
> > > 100644 index 000000000000..7e0466e73980
> > > --- /dev/null
> > > +++ b/drivers/clk/clk-ast2700.c
> > > @@ -0,0 +1,1198 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (c) 2024 ASPEED Technology Inc.
> > > + * Author: Ryan Chen <ryan_chen@xxxxxxxxxxxxxx> */
> > > +
> > > +#include <linux/bits.h>
> > > +#include <linux/clk-provider.h>
> > [...]
> > > +
> > > +struct ast2700_reset {
> > > + void __iomem *base;
> > > + struct ast2700_reset_signal const *signal;
> > > + struct reset_controller_dev rcdev; };
> >
> > Please move the reset controller to the drivers/reset directory by means of
> > using an auxiliary device. There are some existing examples in there if you
> > grep for auxiliary_device in drivers/reset to help guide.
>
> Do you mean to have another reset controller driver?
> If yes, I may need syscon for remap two drivers.
Yes. A syscon is not necessary to do that.
> >
> > > +
> > > +#define to_rc_data(p) container_of(p, struct ast2700_reset, rcdev)
> > > +
> > [...]
> > > +
> > > +static int ast2700_soc0_clk_init(struct device_node *soc0_node) {
> > > + struct clk_hw_onecell_data *clk_data;
> > > + void __iomem *clk_base;
> > [...]
> > > + 0, clk_base +
> > SCU0_CLK_STOP,
> > > + 28, 0,
> > > + &ast2700_clk_lock);
> > > +
> > > + of_clk_add_hw_provider(soc0_node, of_clk_hw_onecell_get,
> > > + clk_data);
> > > +
> > > + return 0;
> > > +};
> > > +
> > > +CLK_OF_DECLARE_DRIVER(ast2700_soc0, "aspeed,ast2700-scu0",
> > > +ast2700_soc0_clk_init); CLK_OF_DECLARE_DRIVER(ast2700_soc1,
> > > +"aspeed,ast2700-scu1", ast2700_soc1_clk_init);
> >
> > Why can't this be a platform driver?
> Due to clk and reset will be the first driver in core.
> Do you think all drivers should be platform driver?
Yes all drivers should be a platform driver.
