From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5C0498002A; Tue, 19 Mar 2024 13:09:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.196 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710853774; cv=none; b=W679tqFepCSetbGeQJO4wMDdEAqqsCpcaMxLXZ55OChErigY5oKl/hdsQA5g9B5IlpbNbjioKczw5U4Rf2z/3C8de/BrlByhMFTgrErjFAke+gl+i9bV2uKETpTTW7HgbQy6yOJC1I1FEbWpxSQCWJx9O+E+rR13fxnTaVH71c0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710853774; c=relaxed/simple; bh=jl5LIVHTbgoOfLHubgBGwWwHndy0u8nCAeBqkZeMR8M=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=KOGX7nFZq+wBD/z8c7X7IkYwFskh8plCXgqGCgI74gKNmPAILFMd6OnMbOZPEMEqiqpMvXzFLn6Su5dz8552vWydi3qtvaLHU6CHd24uf9KZmZMATmNSwhr5kxj+/NpCxuOcRi1ala+WjJENKZALgpz+wYZhwqj9nmYKzmn1lIc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=AkyV0dCr; arc=none smtp.client-ip=217.70.183.196 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="AkyV0dCr" Received: by mail.gandi.net (Postfix) with ESMTPSA id 397FFE0002; Tue, 19 Mar 2024 13:09:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1710853767; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=U/nxcDTUYi1MXeFeU8TRWO7c9qe7Uu/RNWXFdRywHJE=; b=AkyV0dCrSMZ5ix+ksfm7ahaR5LV1hr425kHRYfiYFD9E4NRgvOqZ2YXS8+33sUjznAkmU1 p6C4vkY5NnTysIenRqOuW4YRr+eUrDnKE3CRgRrB/buDV18fY8aHGe94NNW6iqBzCJEOTX +krpiYxIwbS3gIHDHyOm+1FG3QXx+zwTKK8Aq8tUXDCVV+iqOGBxDAZWCRzUzjIgqoxYiI zkxE6+GygI5phLXHzRMx79MTdpqJEFnbxubGKrzfRSbA7gzhtZ5w3Vi5FEIeGf8nDlAEwe 5RXvhLmnu5XJ+Riwypshhyt/72W2Vu6loM1/m6s2z78BsKfkjqTVwIAt+4Jeow== Date: Tue, 19 Mar 2024 14:09:24 +0100 From: Miquel Raynal To: Md Sadre Alam Cc: , , , , , , , , , , , , , , , , , , , , Subject: Re: [PATCH v4 2/5] drivers: mtd: nand: Add qpic_common API file Message-ID: <20240319140924.167f3063@xps-13> In-Reply-To: <756ccc79-0077-5c23-73e3-bbb82fbfa8b0@quicinc.com> References: <20240308091752.16136-1-quic_mdalam@quicinc.com> <20240308091752.16136-3-quic_mdalam@quicinc.com> <20240315124517.4a546ce9@xps-13> <93b08226-3297-2161-cc7d-d33d839c32f0@quicinc.com> <20240319114316.4b977d93@xps-13> <756ccc79-0077-5c23-73e3-bbb82fbfa8b0@quicinc.com> Organization: Bootlin X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-GND-Sasl: miquel.raynal@bootlin.com Hi, quic_mdalam@quicinc.com wrote on Tue, 19 Mar 2024 17:46:05 +0530: > On 3/19/2024 4:13 PM, Miquel Raynal wrote: > > Hi, > > =20 > >>>> +/** > >>>> + * qcom_offset_to_nandc_reg() - Get the actual offset > >>>> + * @regs: pointer to nandc_reg structure > >>>> + * @offset: register offset > >>>> + * > >>>> + * This function will reurn the actual offset for qpic controller r= egister > >>>> + */ > >>>> +__le32 *qcom_offset_to_nandc_reg(struct nandc_regs *regs, int offse= t) > >>>> +{ > >>>> + switch (offset) { > >>>> + case NAND_FLASH_CMD: > >>>> + return ®s->cmd; > >>>> + case NAND_ADDR0: > >>>> + return ®s->addr0; > >>>> + case NAND_ADDR1: > >>>> + return ®s->addr1; > >>>> + case NAND_FLASH_CHIP_SELECT: > >>>> + return ®s->chip_sel; > >>>> + case NAND_EXEC_CMD: > >>>> + return ®s->exec; > >>>> + case NAND_FLASH_STATUS: > >>>> + return ®s->clrflashstatus; > >>>> + case NAND_DEV0_CFG0: > >>>> + return ®s->cfg0; > >>>> + case NAND_DEV0_CFG1: > >>>> + return ®s->cfg1; > >>>> + case NAND_DEV0_ECC_CFG: > >>>> + return ®s->ecc_bch_cfg; > >>>> + case NAND_READ_STATUS: > >>>> + return ®s->clrreadstatus; > >>>> + case NAND_DEV_CMD1: > >>>> + return ®s->cmd1; > >>>> + case NAND_DEV_CMD1_RESTORE: > >>>> + return ®s->orig_cmd1; > >>>> + case NAND_DEV_CMD_VLD: > >>>> + return ®s->vld; > >>>> + case NAND_DEV_CMD_VLD_RESTORE: > >>>> + return ®s->orig_vld; > >>>> + case NAND_EBI2_ECC_BUF_CFG: > >>>> + return ®s->ecc_buf_cfg; > >>>> + case NAND_READ_LOCATION_0: > >>>> + return ®s->read_location0; > >>>> + case NAND_READ_LOCATION_1: > >>>> + return ®s->read_location1; > >>>> + case NAND_READ_LOCATION_2: > >>>> + return ®s->read_location2; > >>>> + case NAND_READ_LOCATION_3: > >>>> + return ®s->read_location3; > >>>> + case NAND_READ_LOCATION_LAST_CW_0: > >>>> + return ®s->read_location_last0; > >>>> + case NAND_READ_LOCATION_LAST_CW_1: > >>>> + return ®s->read_location_last1; > >>>> + case NAND_READ_LOCATION_LAST_CW_2: > >>>> + return ®s->read_location_last2; > >>>> + case NAND_READ_LOCATION_LAST_CW_3: > >>>> + return ®s->read_location_last3; =20 > >>> > >>> Why do you need this indirection? =20 > >> > >> This indirection I believe is needed by the write_reg_dma function, > >> wherein a bunch of registers are modified based on a starting register. > >> Can I change this in a separate cleanup series as a follow up to this?= =20 > >=20 > > I think it would be cleaner to make the changes I requested first and > > then make a copy. I understand it is more work on your side, so if you > > really prefer you can (1) make the copy and then (2) clean it all. But > > please do it all in this series. =20 > Ok > > =20 > >>>> diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mt= d/nand-qpic-common.h > >>>> new file mode 100644 > >>>> index 000000000000..aced15866627 > >>>> --- /dev/null > >>>> +++ b/include/linux/mtd/nand-qpic-common.h > >>>> @@ -0,0 +1,486 @@ > >>>> +/* SPDX-License-Identifier: GPL-2.0 */ > >>>> +/* > >>>> + * QCOM QPIC common APIs header file > >>>> + * > >>>> + * Copyright (c) 2023 Qualcomm Inc. > >>>> + * Authors: Md sadre Alam > >>>> + * Sricharan R > >>>> + * Varadarajan Narayanan > >>>> + * > >>>> + */ > >>>> +#ifndef __MTD_NAND_QPIC_COMMON_H__ > >>>> +#define __MTD_NAND_QPIC_COMMON_H__ > >>>> + > >>>> +#include > >>>> +#include > >>>> +#include > >>>> +#include > >>>> +#include > >>>> +#include > >>>> +#include > >>>> +#include > >>>> +#include > >>>> +#include =20 > >>> > >>> You really need this? =20 > >> Yes , since some generic structure used here. =20 > >=20 > > Which ones? If this is a common file, you probably should not. =20 > Since we are using this struct qcom_nand_controller { } > for both SPI nand as well as raw nand. In this we are having this > struct nand_controller controller member. Maybe we should not expose qcom_nand_controller at all and just share the minimum bits which are really common. Thanks, Miqu=C3=A8l