On Wed, 9 Sep 2020 at 16:35, Simon Glass <s...@chromium.org> wrote: > > Hi Etienne, > > On Wed, 9 Sep 2020 at 03:58, Etienne Carriere > <etienne.carri...@linaro.org> wrote: > > > > On Tue, 8 Sep 2020 at 17:21, Simon Glass <s...@chromium.org> wrote: > > > > > > Hi Etienne, > > > > > > On Mon, 7 Sep 2020 at 08:50, Etienne Carriere > > > <etienne.carri...@linaro.org> wrote: > > > > > > > > Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c > > > > is used to get clock resources, allowing further clock manipulation. > > > > > > > > Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents. > > > > Add DM test scmi_clocks to test these 3 clocks. > > > > Update DM test sandbox_scmi_agent with load/remove test sequences > > > > factorized by {load|remove}_sandbox_scmi_test_devices() helper > > > > functions. > > > > > > > > Signed-off-by: Etienne Carriere <etienne.carri...@linaro.org> > > > > Cc: Simon Glass <s...@chromium.org> > > > > Cc: Peng Fan <peng....@nxp.com> > > > > Cc: Sudeep Holla <sudeep.ho...@arm.com> > > > > --- > > > > > > > > Changes in v3: > > > > - New commit in the series, addresses review comments on test support. > > > > ut_dm_scmi_clocks test SCMI are found and behave as expected for the > > > > implemented clk uclass methods. > > > > --- > > > > arch/sandbox/dts/test.dts | 15 ++ > > > > arch/sandbox/include/asm/scmi_test.h | 37 ++++ > > > > configs/sandbox_defconfig | 1 + > > > > drivers/firmware/scmi/Makefile | 2 +- > > > > drivers/firmware/scmi/sandbox-scmi_agent.c | 169 ++++++++++++++++++- > > > > drivers/firmware/scmi/sandbox-scmi_devices.c | 86 ++++++++++ > > > > test/dm/scmi.c | 139 ++++++++++++++- > > > > 7 files changed, 438 insertions(+), 11 deletions(-) > > > > create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c > > > > > > Reviewed-by: Simon Glass <s...@chromium.org> > > > > > > Nits below > > > > > > [..] > > > > > + * memory allocation to ease sharing with test sequence implementation. > > > > + */ > > > > +static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT]; > > > > +static struct sandbox_scmi_devices sandbox_scmi_devhld; > > > > > > This should really be in a struct, I think, pointed to by > > > dev_get_priv() on this device. I do try to avoid BSS with driver > > > model, although it is not a hard rule with test code. > > > > I used a static structure here to ease sharing context reference with > > the test sequence implementation. > > Is that in a different file?
I made the helper function sandbox_scmi_devices_ctx() exported by sandbox-scmi_devices.c that references probed devices. The function is called from test/dm/scmi.c to run the test (actions & verifications). > > > Context reference returned by sandbox_scmi_devices_ctx() is always > > reliable for the sequence. > > (not possibly dereference in which case the test may segfault). > > > > I can go this way if you prefer no BSS in drivers (Note this is a > > sandbox driver). I'll update the test accordingly. > > If you have a reason to use BSS for a sandbox driver that is OK. I > don't quite understand the problem you are solving though. If private > data is allocated too late, you can allocate the info as platdata > which happens when the device is bound, and then use > dev_get_platdata(). > > But ultimately it is up to you, as this is a test driver. It just > helps to avoid people copying a pattern to their own driver. Ok. I'll try to keep it simple. Regards, etienne > > [..] > > Regards, > Simon