Received-SPF: pass (google.com: domain of linux-kernel+bounces-94846-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Wed, 6 Mar 2024 21:02:38 -0500
From: Frank Li <Frank.li@nxp.com>
To: Rob Herring <robh@kernel.org>
Cc: Conor Dooley <conor@kernel.org>, Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>, Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	Shengjiu Wang <shengjiu.wang@nxp.com>, linux-sound@vger.kernel.org,
	devicetree@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one
 dma-names
Message-ID: <ZekgPg2nsLIQpvY/@lizhi-Precision-Tower-5810>
References: <20240305-asrc_8qxp-v4-0-c61b98046591@nxp.com>
 <20240305-asrc_8qxp-v4-3-c61b98046591@nxp.com>
 <20240306-pebble-grope-88fdaa95a87c@spud>
 <ZejDQddMp17RD6Yk@lizhi-Precision-Tower-5810>
 <20240306205524.GB587561-robh@kernel.org>
 <Zejdz0BKwSlsio9S@lizhi-Precision-Tower-5810>
 <CAL_JsqJw5W7a9Pz9gRPuP717onHdSwRvwGJ=v+QDSe+4ORvTaA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAL_JsqJw5W7a9Pz9gRPuP717onHdSwRvwGJ=v+QDSe+4ORvTaA@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?VFVHYTEzYVZHbERDcEdKZ1NOT2pEVDRPbGpGS0dmc0V5SWVHRlNPcWlFY1Yv?=
 =?utf-8?B?MHNxU25rTVpPTnRBMjM5YWNIVWNEanhka1BWcXRIODhoVjMxbWV0RktCYzNR?=
 =?utf-8?B?U011bU4wWGxZeVhhbllaRzZLWVBPcTl2d2J5bUp5clN4UEhwUzFOR0xKQ3JO?=
 =?utf-8?B?V0RzQXRKY21xSEtqQUFOV3lQM0xLSi8vMzdnaVM2WlhUcWZsN216cWl1SnBL?=
 =?utf-8?B?RXBBVHVjbEc4L3d6MjJHMmtzWVZKbmxDSEhHSEc4TFNreVVmWmJYQ3hiV1Rq?=
 =?utf-8?B?MkpJQXRsWHUzK2piRXgyVXYvcUtQUFFpck50VUlaQVQ4OVIvMUVLY3c5a0R2?=
 =?utf-8?B?YjFGTnVtMVR1Vzc0OWZORjFBRFlPbmR3VDlzd255OTBwbC9WOVBRK2k4M2E4?=
 =?utf-8?B?OWJsdGNkclhTN1lNb1dLc29Qd0FrK0ZhbUcxVW96UFIxMjlsOERTdUxKNlBB?=
 =?utf-8?B?OXRPL2NwMGY4SUV2RzQ0RTEyU2w4YURtUndTNlh1ZlVyekx0WUxMNWZUZWt5?=
 =?utf-8?B?Nmd2anVlbEhMWVlBNXV2cTFwSUlHR3hXazlELytuT2cvY09XaWs4TU0rVzds?=
 =?utf-8?B?YzQ3WFBRWk1QWThEVXJ5bXZzR1BURUtobk1wcDBjL2ZaYnFmN3cxbFBPL0I2?=
 =?utf-8?B?QTVMbHErYnJNdzVXTHBCZDNzY3R2V3FWZ2RWNDFiaDhlcG9xR09BbE1NNEdi?=
 =?utf-8?B?U3hxbHpQQ3g1eHNjUnEyVmlTZ3pyaEJuWlcrOU9reUphTzJINmtZY0x0dGli?=
 =?utf-8?B?YTFNVVVNUndaUG9UVlJVSXBrYTJNZW8zZVJyL3I4KzNRLzFVSG9obUxONGNH?=
 =?utf-8?B?cS8xWUwrZVUrK25NRW83L2tJUmR5ZXozLzRqS25TTHQ5djVLZEgvWk9sOExj?=
 =?utf-8?B?elptbEM3eTVkcW15N1hkVjU3VVNaU3VXbFhQZDUzQTBOTHZnenRIbDFPRW5v?=
 =?utf-8?B?YlR4YXhZQ0ZNWkVTVWRGVVhwbXYyUmFrT1dkT1JrV1lYcVgwREtVbzhHM3JT?=
 =?utf-8?B?N3NJQlhFWmZPc0FtaE9mR2Fsekp1TXV0V0g3ODNFNW94Q0VkZVgzR2tVd2ti?=
 =?utf-8?B?aUVuekpMaVhORHFYNjdHRERVRWVDT0dGTnUvNXBBT1BEdEFnVmFpUHcyUWo1?=
 =?utf-8?B?RUxtVkVGd3l0Tk4zVlVHRjJTOTFaS1JsQjBqbzBseVgzckp2TitWb3lmRS94?=
 =?utf-8?B?WTIreW04NzVIR1pXamZWTWxROCtIeVhwNWxuYkZjRFVZVDlNdjhGMUFQNG92?=
 =?utf-8?B?bHgzekV3UEx0Vnd2Ni9KODV1THVFSUhNQTdnTUFSSFRGZWZmTjE2OUJYMTJS?=
 =?utf-8?B?cnV3ek8xd1lpczliSGpLT05aZ1VKdnJLdVY2anNlMiszZzdLNVh5ZklzZkRQ?=
 =?utf-8?B?Sm4vSVlhWGgzUXlNdGVaenNUMWJUdk83bXlQWWJuZVpzREdsWUllNzV0a1Q4?=
 =?utf-8?B?RnhaZFVINHYwWjM3bHhzcU9ITEhtN3czNG4waUNxT1VZSnNGNkJnSXE5a2NL?=
 =?utf-8?B?NnBFUTZSQTZiV0kxb1FIeG42ZkFiaFdyM3VTRjJVazRKYmFHeDZvWlhsYnJp?=
 =?utf-8?B?YzR1aXBIRkNwdXdHM0RONGR1M1dDOGQrUk4rT3RLdStDVkd1M2xjbUxuYWNS?=
 =?utf-8?B?UVo2YXNIVFgyQlRnSnJrY3p2VXp6NnBuZndwZ0I4aENDdHlGSWZyc3dHUFNT?=
 =?utf-8?B?R3M5Tkxzbmp6ZFl6QkMwOXQ0OHh1TGkwQjF4bHdZeE9GeFNUeE5lUVFrcDky?=
 =?utf-8?B?bUNuRktFbjE5ZVFSRXduaEN5TllyWGZ0VVJ6WlpqdWpDNlQ0VDJWUGxQdFZO?=
 =?utf-8?B?dnl6VEJaUU1IOU5uSXVZM0NZcFVUZ1RtenRaT0wrVnBSYzVDWUxQMFlRZytX?=
 =?utf-8?B?ZGpGUzVPTUhmV05SMXBOZmhGb0FKcGdOenNNcS8zcUVNeC9GcUMwVitZTTA5?=
 =?utf-8?B?SjA3QWF6bnJGN0VqSkVvK3hjdkVPNUlNNlEzNGVocGcxeXI1T3crK1FNQ001?=
 =?utf-8?B?dWtUaEt6QUpJQU9xcFpZRUpjM1Jlc2ZtSnFEMVpFL1VPOWFXMU84OEh1dWJ4?=
 =?utf-8?B?eGYrSUFOcEdwZHcvTUtuRzN6aE4wcWVuU2VDdjNaa0cwQklzZFJIeWUxeTdm?=
 =?utf-8?Q?m/2U=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6cf1d107-cd61-4d41-b4b4-08dc3e4aafda
X-MS-Exchange-CrossTenant-AuthSource: DB9PR04MB9626.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2024 02:02:48.0614
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 0dc8GQgsETCXxKSAsIBe0m587x+DWg+o95o1KMNBzcEcNgSur0kAWhvqFGU7K9y8DiO10KF7ojYXZOOvwHn49A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR04MB8461

On Wed, Mar 06, 2024 at 04:58:22PM -0600, Rob Herring wrote:
> On Wed, Mar 6, 2024 at 3:19 PM Frank Li <Frank.li@nxp.com> wrote:
> >
> > On Wed, Mar 06, 2024 at 02:55:24PM -0600, Rob Herring wrote:
> > > On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> > > > On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > > > > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > > > for dma-names.
> > > > > >
> > > > > > Remove description under dmas because no user use index to get dma channel.
> > > > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > > > 'required' list.
> > > > >
> > > > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > >
> > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > > > >  1 file changed, 6 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > index 2456d958adeef..6f551c68d33db 100644
> > > > > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > @@ -81,15 +81,14 @@ properties:
> > > > > >
> > > > > >    dmas:
> > > > > >      minItems: 1
> > > > > > -    items:
> > > > > > -      - description: DMA controller phandle and request line for RX
> > > > > > -      - description: DMA controller phandle and request line for TX
> > > > > > +    maxItems: 2
> > > > > >
> > > > > >    dma-names:
> > > > > > -    minItems: 1
> > > > > > -    items:
> > > > > > -      - const: rx
> > > > > > -      - const: tx
> > > > > > +    oneOf:
> > > > > > +      - items:
> > > > > > +          - const: rx
> > > > > > +          - const: tx
> > > > > > +      - enum: [ rx, tx ]
> > > > >
> > > > > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > > > > he was suggesting that in the two items case we'd not care about the
> > > > > order. But while I think this is different to that suggestion it's also
> > > > > not wrong.
> > > >
> > > > I log this at cover-letter. b4 can't support write change log at every
> > > > patch yet.
> > >
> > > It never will (probably). That's because it doesn't need to. You can
> > > just do it with git. When you edit the commit message, then after the
> > > tags, Add '---' and put whatever you want after. That works as long as
> > > the commit is applied from a patch as 'git am' will drop it.
> >
> > The key problem is that I don't want to lost notes when respin patches. It
> > is easy to make mistake when I copy old serise change logs.
> 
> You don't. It is all saved in the commit. When you rebase for the next
> version, you just add to the changes history.
> 
> > Previously I use git notes + git-rebase, it work fine. Notes can be kept
> > when I do rebase and git commit --amend. But one thing is not good.
> > git send-email --to-cmd=./script/get_maintainer.sh *.patch. It can't
> > combine all patches's maitainer to a list. It looks like difference patch
> > will be difference --to list.
> >
> > b4 can help some case. But can't keep git-notes information when rebase.
> > It should be git bugs or feature missed.
> 
> git-notes always seemed clunky and to be extra work to me. Just having
> the changelog in the commit msg is a lot easier.
> 
> > > > Rob's suggest was not work. dt-binding check complain too long
> > > > if there are two dma-names = "rx", "tx".
> > >
> > > So I'm wrong or you didn't have it correct? No way to tell with your
> > > explanation. Let me give you the exact schema:
> > >
> > > dma-names:
> > >   minItems: 1
> > >   items:
> > >     - enum: [ rx, tx ]
> > >     - const: tx
> > >
> > > This says we can have 1 or 2 entries. The first entry can be either rx
> > > or tx. The 2nd entry must be tx. That's what you want. However, '"tx",
> > > "tx"' is allowed with the above, but we enforce items to be unique
> > > elsewhere. Or I thought we did, but we relaxed '.*-names$' at some
> > > point. I'm going to fix that now.
> >
> > Conor find out my problem. The below code works. I missed maxItems.
> >
> >   dma-names:
> >      minItems: 1
> >      maxItems: 2
> >      items
> 
> Missing ':'
> 
> >        enum: [ rx, tx ]
> 
> That is not my suggestion. This would be my 3rd choice after what I
> proposed or what you had already. Please plug in exactly what I told
> you and report back what doesn't work.

It also works. look likes conor's method is a little bit easy to be
understood.

Frank
> 
> Rob