Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 3 Nov 2021 12:48:47 -0500
From:   Bjorn Andersson <bjorn.andersson@linaro.org>
To:     Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com>
Cc:     Ohad Ben-Cohen <ohad@wizery.com>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-stm32@st-md-mailman.stormreply.com, julien.massot@iot.bzh
Subject: Re: [PATCH v6 02/10] rpmsg: create the rpmsg class in core instead
 of in rpmsg char
Message-ID: <YYLLf1/R5gJJfo/H@builder.lan>
References: <20211022125426.2579-1-arnaud.pouliquen@foss.st.com>
 <20211022125426.2579-3-arnaud.pouliquen@foss.st.com>
 <YYAcYLxEmbwJShg7@builder.lan>
 <962ca426-5cc4-4adb-df55-27fe93f7e767@foss.st.com>
 <YYFpit0SixojReo+@ripper>
 <16853c3d-0b23-4d59-9c33-c7c99da4b9a1@foss.st.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <16853c3d-0b23-4d59-9c33-c7c99da4b9a1@foss.st.com>
Precedence: bulk

On Tue 02 Nov 12:11 CDT 2021, Arnaud POULIQUEN wrote:
> On 11/2/21 5:38 PM, Bjorn Andersson wrote:
> > On Tue 02 Nov 08:23 PDT 2021, Arnaud POULIQUEN wrote:
> >> On 11/1/21 5:57 PM, Bjorn Andersson wrote:
> >>> On Fri 22 Oct 07:54 CDT 2021, Arnaud Pouliquen wrote:
[..]
> >>>> @@ -296,6 +298,19 @@ __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,
> >>>>  }
> >>>>  EXPORT_SYMBOL(rpmsg_poll);
> >>>>  
> >>>> +/**
> >>>> + * rpmsg_get_class() - get reference to the sysfs rpmsg class
> >>>> + *
> >>>> + * This function return the pointer to the "rpmsg" class created by the rpmsg core.
> >>>> + *
> >>>> + * Returns the struct class pointer
> >>>> + */
> >>>> +struct class *rpmsg_get_class(void)
> >>>
> >>> What value does this helper function add? Can't we just expose
> >>> rpmsg_class directly?
> >>
> >> look to me cleaner to not expose directly the rpmsg_class in rpmsg.h as this
> >> variable is read only for rpmsg services.
> >>
> > 
> > The pointer is read only, but the object isn't. So I think it's cleaner
> > to just share the pointer in the first place.
> > 
> > But that said, looking at this a little bit more, I don't think there's
> > any guarantee that class_create() has been executed before
> > rpmsg_ctrl_probe() is being invoked.
> 
> class_create is called in in the rpmsg_init (rpmsg_core.c). as RPMSG_CTRL and
> RPMSG_CHAR depend on RPMSG config this use case should not occurs, or did I miss
> something?
> 

Specifying "depends on RPMSG" in the Kconfig doesn't directly impact the
load order.

But in expanding my question I realized that rpmsg_ctrl_probe() will
only ever be invoked if rpmsg_ctrldev_init() was able to call
register_rpmsg_drive(), which in turn depends on rpmsg_init() having
initialized rpmsg_bus of the driver_register() will fail.

So we should be good.

Thanks,
Bjorn