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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241025094825.767c84c7@xps-13>
Date: Fri, 25 Oct 2024 09:48:25 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Martin Kurbanov <mmkurbanov@...utedevices.com>
Cc: Richard Weinberger <richard@....at>, Vignesh Raghavendra
 <vigneshr@...com>, Mika Westerberg <mika.westerberg@...ux.intel.com>,
 "Michael Walle" <michael@...le.cc>, Mark Brown <broonie@...nel.org>,
 Chia-Lin Kao <acelan.kao@...onical.com>, Md Sadre Alam
 <quic_mdalam@...cinc.com>, "Ezra Buehler"
 <ezra.buehler@...qvarnagroup.com>, Sridharan S N <quic_sridsn@...cinc.com>,
 Frieder Schrempf <frieder.schrempf@...tron.de>, Alexey Romanov
 <avromanov@...utedevices.com>, <linux-kernel@...r.kernel.org>,
 <linux-mtd@...ts.infradead.org>, <kernel@...utedevices.com>
Subject: Re: [PATCH v2 2/5] mtd: spinand: add OTP support

Hi Martin,

Sorry for the slow feedback.

> >> +/**
> >> + * spinand_set_mtd_otp_ops() - Set up OTP methods
> >> + * @spinand: the spinand device
> >> + *
> >> + * Set up OTP methods.
> >> + */
> >> +void spinand_set_mtd_otp_ops(struct spinand_device *spinand)
> >> +{
> >> +	struct mtd_info *mtd = spinand_to_mtd(spinand);
> >> +
> >> +	if (!spinand->otp->ops)  
> > 
> > Could we use something else as check? It feels odd to check for otp ops
> > and then just ignore the fact that they are here. Maybe check npages or
> > otp_size() ?  
> 
> A developer may not specify OTP callbacks:
> SPINAND_OTP_INFO(otp_pages, NULL /* OTP ops */)

Is this really a valid situation?

In set_mtd_otp_ops() you set spinand functions only if there are otp
operations. First, is it relevant to consider the fact that a device
would have an otp and not provide operations? Otherwise, my initial
comment was about the fact that the check seems uncorrelated with the
second part of the function.

Maybe setting these functions only if relevant is the best choice, so
you no longer have to make the checks after the init.

	if (ops && ops->erase)
		mtd->_otp_erase = spinand_otp_erase;
	...

?

Thanks,
Miquèl

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ