On Tue, 2024-11-19 at 09:19 +0800, Xu Yilun wrote: > 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. Thank you for the clarification. This and the subsequent patch have been merged in [PATCH v5 15/18], to avoid a window between binfo->type already holding a valid DFL id but binfo->feature_dev still NULL. Peter > > > 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 > > > > > > > > > >
