On Mon, Nov 18, 2024 at 11:00:28PM +0000, Colberg, Peter wrote: > On Mon, 2024-11-18 at 22:32 +0800, Xu Yilun wrote: > > On Fri, Oct 25, 2024 at 06:37:10PM -0400, Peter Colberg wrote: > > > Use binfo->type instead of binfo->feature_dev to decide whether a > > > feature device was detected during feature parsing. A subsequent > > > commit will delay the allocation of the feature platform device > > > to feature_dev_register() and remove binfo->feature_dev. > > > > > > This commit does not introduce any functional changes. > > > > > > Signed-off-by: Peter Colberg <peter.colberg@xxxxxxxxx> > > > Reviewed-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx> > > > Reviewed-by: Basheer Ahmed Muddebihal <basheer.ahmed.muddebihal@xxxxxxxxxxxxxxx> > > > --- > > > Changes since v3: > > > - New patch extracted from last patch of v3 series. > > > --- > > > drivers/fpga/dfl.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c > > > index 758673b0290a..a9ec37278b2d 100644 > > > --- a/drivers/fpga/dfl.c > > > +++ b/drivers/fpga/dfl.c > > > @@ -1248,7 +1248,7 @@ static int parse_feature_port_afu(struct build_feature_devs_info *binfo, > > > return create_feature_instance(binfo, ofst, size, FEATURE_ID_AFU); > > > } > > > > > > -#define is_feature_dev_detected(binfo) (!!(binfo)->feature_dev) > > > +#define is_feature_dev_detected(binfo) ((binfo)->type != DFL_ID_MAX) > > > > I still doesn't get why put the change here. How it resolves my concern > > compared to v3? > > Could you elaborate on your concern? I moved this change into a > separate commit so that it is not lost in other changes, but I don't I did't mean this change had to be separated, I doubt it impacts the functionality when it was applied. After the series were all applied the issue may be fixed but people review patches one by one. > see how the two definitions would not be functionally equivalent. Would > it help to extend the commit description along the following lines? > > 1. Before this series, binfo->feature_dev was initialized to NULL by > devm_kzalloc() in dfl_fpga_feature_devs_enumerate(). After this > series, binfo->type is initialized to DFL_ID_MAX in > dfl_fpga_feature_devs_enumerate(). > 2. Before this series, binfo->feature_dev was set to a non-NULL > pointer in build_info_create_dev(), which in turn was called from So at the point of *this patch* is applied, binfo->feature_dev & binfo->type don't initialize at the same time, there is some gap the caller of is_feature_dev_detected() would get a different result, e.g. when build_info_create_dev() fails, binfo->type holds valid DFL_ID but binfo->feature_dev is NULL. > parse_feature_fiu(). After this series, binfo->type is set to a > non-DFL_ID_MAX value, as returned by dfh_id_to_type(), in > parse_feature_fiu(). > 3. Before this series, binfo->feature_dev was reset to NULL at the > end of build_info_commit_dev(). After this series, binfo->type is > reset to DFL_ID_MAX at the end of build_info_commit_dev(). > > Thanks, > Peter > > > > > Thanks, > > Yilun > > > > > > > > static int parse_feature_afu(struct build_feature_devs_info *binfo, > > > resource_size_t ofst) > > > -- > > > 2.47.0 > > > > > > >
