Re: [PATCH V3 3/3] mailbox: qcom-apcs: Add SM6125 compatible

[Rob Herring]

On Mon, Jun 21, 2021 at 5:10 PM Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
>
> On Mon, Jun 21, 2021 at 2:46 PM Rob Herring <robh+dt@xxxxxxxxxx> wrote:
> >
> > On Sun, Jun 20, 2021 at 10:03 PM Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
> > >
> > > On Sat, Jun 12, 2021 at 4:46 AM Martin Botka
> > > <martin.botka@xxxxxxxxxxxxxx> wrote:
> > > >
> > > > This commit adds compatible for the SM6125 SoC
> > > >
> > > > Signed-off-by: Martin Botka <martin.botka@xxxxxxxxxxxxxx>
> > > > ---
> > > > Changes in V2:
> > > > None
> > > > Changes in V3:
> > > > Change compatible to apcs-hmss-global
> > > >  drivers/mailbox/qcom-apcs-ipc-mailbox.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> > > > index f25324d03842..f24c5ad8d658 100644
> > > > --- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> > > > +++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
> > > > @@ -57,6 +57,10 @@ static const struct qcom_apcs_ipc_data sdm660_apcs_data = {
> > > >         .offset = 8, .clk_name = NULL
> > > >  };
> > > >
> > > > +static const struct qcom_apcs_ipc_data sm6125_apcs_data = {
> > > > +       .offset = 8, .clk_name = NULL
> > > > +};
> > > > +
> > > >  static const struct qcom_apcs_ipc_data apps_shared_apcs_data = {
> > > >         .offset = 12, .clk_name = NULL
> > > >  };
> > > > @@ -166,6 +170,7 @@ static const struct of_device_id qcom_apcs_ipc_of_match[] = {
> > > >         { .compatible = "qcom,sc8180x-apss-shared", .data = &apps_shared_apcs_data },
> > > >         { .compatible = "qcom,sdm660-apcs-hmss-global", .data = &sdm660_apcs_data },
> > > >         { .compatible = "qcom,sdm845-apss-shared", .data = &apps_shared_apcs_data },
> > > > +       { .compatible = "qcom,sm6125-apcs-hmss-global", .data = &sm6125_apcs_data },
> > > >         { .compatible = "qcom,sm8150-apss-shared", .data = &apps_shared_apcs_data },
> > > >         { .compatible = "qcom,sdx55-apcs-gcc", .data = &sdx55_apcs_data },
> > > >         {}
> > > >
> > > These all are basically different names for the same controller.
> > > The 'offset' is a configuration parameter and the 'clock', when NULL,
> > > is basically some "always-on" clock.
> > > I am sure we wouldn't be doing it, if the controller was third-party.
> >
> > If newer implementations are 'the same', then they should have a
> > fallback compatible to the existing one that is the same and no driver
> > change is needed. If the differences are board or instance (within an
> > SoC) specific, then a DT property would be appropriate.
> >
> The controllers (13 now) only differ by the 'offset' where the
> registers are mapped. Clock-name is a pure s/w artifact.
> So, maybe we could push all these in DT.

Why is 'reg' not used for the offset?

In any case, we can't really get rid of the first 13 instances though...

Rob