Re: [PATCHv3 0/5] FWU: Handle meta-data in common code

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3258 bytes --]

On Wed, Jan 18, 2023 at 08:13:01AM -0600, Jassi Brar wrote:
> On Wed, Jan 18, 2023 at 7:28 AM Michal Simek <michal.simek@amd.com> wrote:
> >
> > Hi,
> >
> > On 1/2/23 19:25, Jassi Brar wrote:
> > > The patchset reduces ~400 lines of code, while keeping the functionality same and making
> > > meta-data operations much faster (by using cached structures).
> > >
> > > Issue:
> > >   meta-data copies (primary and secondary) are being handled by the backend/storage layer
> > > instead of the common core in fwu.c (as also noted by Ilias)  that is, gpt_blk.c manages
> > > meta-data and similarly raw_mtd.c will have to do the same when it arrives. The code
> > > could by make smaller, cleaner and optimised.
> > >
> > > Basic idea:
> > >   Introduce  .read_mdata() and .write_mdata() in fwu_mdata_ops  that simply read/write
> > > meta-data copy. The core code takes care of integrity and redundancy of the meta-data,
> > > as a result we can get rid of every other callback .get_mdata() .update_mdata()
> > > .get_mdata_part_num()  .read_mdata_partition()  .write_mdata_partition() and the
> > > corresponding wrapper functions thereby making the code 100s of LOC smaller.
> > >
> > > Get rid of fwu_check_mdata_validity() and fwu_mdata_check() which expected underlying
> > > layer to manage and verify mdata copies.
> > > Implement  fwu_get_verified_mdata(struct fwu_mdata *mdata) public function that reads,
> > > verifies and, if needed, fixes the meta-data copies.
> > >
> > > Verified copy of meta-data is now cached as 'g_mdata' in fwu.c, which avoids multiple
> > > low-level expensive read and parse calls.
> > > gpt meta-data partition numbers are now cached in gpt_blk.c, so that we don't have to do expensive part_get_info() and uid ops.
> >
> > First of all I have strong suspicious that this series are pretty much two
> > series at once.
> >
> Yes, I submitted two patchsets.
> 1) Optimizing the api of current fwu.
> 2) Introduce support for mtd backed storage (DeveloperBox platform as
> an instance) using the new api.
> 
> They appear just fine in my inbox. Do they appear bad to you?
> 
> >
> > The second issue is that you are sending patches from
> > Jassi Brar <jassisinghbrar@gmail.com>
> > but SOB is
> > Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> >
> > And Tom said in past that they should match. There is a hook for it to check it
> > which everybody should be using. That's why please fix this in the next series.
> >
> I have submitted dozens of patches and pull requests over the last
> many years. This never occurred to anybody.
> BTW, the 'Author' and 'Signed-off-by' appear consistent in git log.
> And there are very recent instances in uboot git log where even they
> actually differ.
> 
> But if Tom really wants, I am happy to send-email from my other account.

It doesn't matter what the email you send from is, but Author and Signed-off-by
really really should match in the commit itself. It can be annoying,
depending on corporate setup / policies, to have send-email work
directly with your corporate account, but if you're allowed to be
submitting patches, that's where the match between Author and
Signed-off-by matters.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]