linux-kernel - Re: [PATCH v3 8/9] mtd: rawnand: ingenic: Add support for the JZ4725B

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20190205195457.171cd3e3@xps13>
Date:   Tue, 5 Feb 2019 19:54:57 +0100
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Paul Cercueil <paul@...pouillou.net>
Cc:     David Woodhouse <dwmw2@...radead.org>,
        Brian Norris <computersforpeace@...il.com>,
        Boris Brezillon <bbrezillon@...nel.org>,
        Marek Vasut <marek.vasut@...il.com>,
        Richard Weinberger <richard@....at>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Harvey Hunt <harveyhuntnexus@...il.com>,
        Mathieu Malaterre <malat@...ian.org>,
        linux-mtd@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 8/9] mtd: rawnand: ingenic: Add support for the
 JZ4725B

Hi Paul,


[...]

> >>  +
> >>  +static void jz4725b_bch_init(struct ingenic_ecc *bch,
> >>  +			     struct ingenic_ecc_params *params, bool encode)  
> > 
> > I don't know the IP but 'encode' looks strange, what is it supposed to
> > mean?  
> 
> It is used to toggle between calculating the ECC codes for a given set of data,
> and using supplied ECC codes to verify a set of data.
> 
> I just reused the function and parameter names that were used in the
> jz4780-bch driver, that's where the 'encode' comes from.


I see, so it is kind of a "derive" vs. "verify" information. Please
add a comment explaining the parameter name then.

> >>  +{
> >>  +	u32 reg;
> >>  +
> >>  +	/* Clear interrupt status. */
> >>  +	writel(readl(bch->base + BCH_BHINT), bch->base + BCH_BHINT);
> >>  +
> >>  +	/* Initialise and enable BCH. */
> >>  +	writel(0x1f, bch->base + BCH_BHCCR);
> >>  +	writel(BCH_BHCR_BCHE, bch->base + BCH_BHCSR);
> >>  +
> >>  +	if (params->strength == 8)
> >>  +		writel(BCH_BHCR_BSEL_MASK, bch->base + BCH_BHCSR);
> >>  +	else
> >>  +		writel(BCH_BHCR_BSEL_MASK, bch->base + BCH_BHCCR);  
> > 
> > Here you write to BCH_BHCSR or BCH_BHCCR depending on
> > params->strength...
> >   
> >>  +
> >>  +	if (encode)
> >>  +		writel(BCH_BHCR_ENCE, bch->base + BCH_BHCSR);
> >>  +	else
> >>  +		writel(BCH_BHCR_ENCE, bch->base + BCH_BHCCR);  
> > 
> > ...and here depending on encode.
> > 
> > Can you explain a bit?  
> 
> BCH_BHCSR / BCH_BHCCR are set and clear registers for the BCH_BHCR
> register, which is itself read-only. The BSEL field is used to toggle
> between a strength of 4 (if cleared) and 8 (if set), while the ENCE
> field configures the IP for 'encoding' mode.

All clear. Can you add a short comment to explain it somewhere?


Thanks,
Miquèl