linux-kernel - Re: [RFC PATCH v2 2/4] fpga: add fake FPGA bridge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZB6iaKEVnVfV3Atl@yilunxu-OptiPlex-7050>
Date:   Sat, 25 Mar 2023 15:27:36 +0800
From:   Xu Yilun <yilun.xu@...el.com>
To:     Marco Pagani <marpagan@...hat.com>
Cc:     Moritz Fischer <mdf@...nel.org>, Wu Hao <hao.wu@...el.com>,
        Tom Rix <trix@...hat.com>, linux-kernel@...r.kernel.org,
        linux-fpga@...r.kernel.org
Subject: Re: [RFC PATCH v2 2/4] fpga: add fake FPGA bridge

On 2023-03-21 at 18:33:05 +0100, Marco Pagani wrote:
> 
> 
> On 2023-03-17 09:32, Xu Yilun wrote:
> > On 2023-03-10 at 18:04:10 +0100, Marco Pagani wrote:
> >> Add fake FPGA bridge driver with support functions. The driver includes
> >> a counter for the number of switching cycles. This module is part of
> >> the KUnit tests for the FPGA subsystem.
> >>
> >> Signed-off-by: Marco Pagani <marpagan@...hat.com>
> >> ---
> >>  drivers/fpga/tests/fake-fpga-bridge.c | 228 ++++++++++++++++++++++++++
> >>  drivers/fpga/tests/fake-fpga-bridge.h |  36 ++++
> >>  2 files changed, 264 insertions(+)
> >>  create mode 100644 drivers/fpga/tests/fake-fpga-bridge.c
> >>  create mode 100644 drivers/fpga/tests/fake-fpga-bridge.h
> >>
> >> diff --git a/drivers/fpga/tests/fake-fpga-bridge.c b/drivers/fpga/tests/fake-fpga-bridge.c
> >> new file mode 100644
> >> index 000000000000..8a2f64fc1bbb
> >> --- /dev/null
> >> +++ b/drivers/fpga/tests/fake-fpga-bridge.c
> >> @@ -0,0 +1,228 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Driver for the fake FPGA bridge
> >> + *
> >> + * Copyright (C) 2023 Red Hat, Inc.
> >> + *
> >> + * Author: Marco Pagani <marpagan@...hat.com>
> >> + */
> >> +
> >> +#include <linux/types.h>
> >> +#include <linux/device.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/fpga/fpga-bridge.h>
> >> +#include <kunit/test.h>
> >> +
> >> +#include "fake-fpga-bridge.h"
> >> +
> >> +#define FAKE_FPGA_BRIDGE_DEV_NAME	"fake_fpga_bridge"
> >> +
> >> +struct fake_bridge_priv {
> >> +	int id;
> >> +	bool enable;
> >> +	int cycles_count;
> >> +	struct kunit *test;
> >> +};
> >> +
> >> +struct fake_bridge_data {
> >> +	struct kunit *test;
> >> +};
> >> +
> >> +static int op_enable_show(struct fpga_bridge *bridge)
> >> +{
> >> +	struct fake_bridge_priv *priv;
> >> +
> >> +	priv = bridge->priv;
> >> +
> >> +	if (priv->test)
> >> +		kunit_info(priv->test, "Fake FPGA bridge %d: enable_show\n",
> >> +			   priv->id);
> > 
> > Why check the kunit pointer every time? I remember you mentioned that
> > the fake fpga modules are expected to be used out of Kunit test, so the
> > priv->test may be NULL? I suggest you work on these usecases in separate
> > patchsets. For now just check priv->test on probe is fine.
> > 
> 
> The idea was to provide additional info messages, tied with the test, if the
> fake bridge is registered with a test instance. If you believe these prints
> are unnecessary, I can remove them or replace them with generic dev_info().

OK, on second thought, it's good to me.

Thanks,
Yilun