Re: [Patch v7 1/8] memory: tegra: add interconnect support for DRAM scaling in Tegra234

[Sumit Gupta]

On 09/05/23 19:06, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
>
>
> On 09/05/2023 15:17, Sumit Gupta wrote:
> > > > > > > +                     /*
> > > > > > > +                      * MC driver probe can't get BPMP reference as
> > > > > > > it gets probed
> > > > > > > +                      * earlier than BPMP. So, save the BPMP ref got
> > > > > > > from the EMC
> > > > > > > +                      * DT node in the mc->bpmp and use it in MC's
> > > > > > > icc_set hook.
> > > > > > > +                      */
> > > > > > > +                     mc->bpmp = emc->bpmp;
> > > > > >
> > > > > > This (and ()) are called without any locking. You register first the
> > > > > > interconnect, so set() callback can be used, right? Then set() could be
> > > > > > called anytime between tegra_emc_interconnect_init() and assignment
> > > > > > above. How do you synchronize these?
> > > > > >
> > > > > > Best regards,
> > > > > > Krzysztof
> > > > >
> > > > > Currently, the tegra234_mc_icc_set() has NULL check. So, it will give
> > > > > this error.
> > > > >     if (!mc->bpmp) {
> > >
> > > How does it solve concurrent accesses and re-ordering of instructions by
> > > compiler or CPU?
> >
> > Now, the "mc->bpmp" is set before tegra_emc_interconnect_init().
> > So, until the EMC interconnect initializes, set() won't be
> > called as the devm_of_icc_get() call will fail.
>
> What if compiler puts "mc->bpmp" assignment after
> tegra_emc_interconnect_init()?
>
> What if CPU executes above assignment also after
> tegra_emc_interconnect_init()?
>
> Considering amount of code inside tegra_emc_interconnect_init() second
> case is rather unlikely, but first possible, right?
>
> Best regards,
> Krzysztof

Yes, for protection from compiler re-ordering I can add
barrier() after the assignment.

  mc->bpmp = emc->bpmp;
  barrier();

Thank you,
Sumit Gupta