On Sun, Sep 17, 2023 at 6:46 PM Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Sun, 17 Sept 2023 at 02:12, Bartosz Golaszewski <brgl@xxxxxxxx> wrote: > > > > + has_line_names = true; > > + max_offset = max(line->offset, max_offset); > > I really don't understand why you kept this old broken logic. > > I sent a much better version of this function that didn't need that > pointless has_line_names thing or the 'max()' thing, by just making > the code a lot simpler. > Right, it does what it's supposed to after all but IMO it's less clear, I had to take a second look now to get it. I was wondering if I'm simply too sleep deprived but no - it's because in your version the max_offset variable actually holds the value of (max_offset + 1) which makes the name untrue. I don't want to bikeshed about it, let me know if my version is GoodEnough(R) or do you prefer another respin. Bart > Whatever. > > > + line_names_size = gpio_sim_get_line_names_size(bank); > > + if (line_names_size) { > > + line_names = kcalloc(line_names_size, sizeof(*line_names), > > + GFP_KERNEL); > > + if (!line_names) > > + return ERR_PTR(-ENOMEM); > > + > > + gpio_sim_set_line_names(bank, line_names); > > > > - if (line_names) > > properties[prop_idx++] = PROPERTY_ENTRY_STRING_ARRAY_LEN( > > "gpio-line-names", > > line_names, line_names_size); > > + } > > But I do like this reorganization. > > Linus