Received-SPF: pass (google.com: domain of linux-kernel+bounces-87962-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <9258eb2a-8764-4b2a-9c40-de3faf903610@nuvoton.com>
Date: Fri, 1 Mar 2024 14:35:51 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 2/2] ASoC: nau8325: new driver
Content-Language: en-US
To: Mark Brown <broonie@kernel.org>
Cc: lgirdwood@gmail.com, alsa-devel@alsa-project.org,
 devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
 robh+dt@kernel.org, conor+dt@kernel.org, YHCHuang@nuvoton.com,
 KCHSU0@nuvoton.com, CTLIN0@nuvoton.com, SJLIN0@nuvoton.com,
 scott6986@gmail.com, supercraig0719@gmail.com, dardar923@gmail.com
References: <20240222083825.190854-1-wtli@nuvoton.com>
 <20240222083825.190854-3-wtli@nuvoton.com>
 <6ffb4165-95e0-4ccd-868f-8ecda56b4079@sirena.org.uk>
From: WTLI <wtli@nuvoton.com>
In-Reply-To: <6ffb4165-95e0-4ccd-868f-8ecda56b4079@sirena.org.uk>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?ZzAvWGpteHd3OEVGS2ZSRnYyR2djcUkvbkxIR0JRc2Rad3NtUWd6MW9JWkhs?=
 =?utf-8?B?QjlDSEtwS2xwRWpCbVZPUFNTZ2NwS1VUbFhRc1NpclJQUE9DZzZSeENrdjFW?=
 =?utf-8?B?YnJMZEFHbHNVRzM5cGdYVUhQRTR4WUNOMGtoa3A3am9RTlFrYmE3L1h3cW5Q?=
 =?utf-8?B?MWlaUE9UbUZLS0kyMURJbmRVSFJ3bmZMY1FLa2Q2M0g5blNwazl1YmZmZUFp?=
 =?utf-8?B?SlBta1k4VDkweElsd1RTRDg0VFBkSmVMWjVxTE5RbENYOXNoTGhvN05RYnFp?=
 =?utf-8?B?YVBvazlycm5tV29sWVY3WDR0T1oxckFZL1R3eUZOd2hIV0QxemlLemJpMlBu?=
 =?utf-8?B?OWNrbzRHS3kzV2FJc01kWWtmejYyZkQyTGF2RzZoYkxzNWdtNTRnNTBRQUVG?=
 =?utf-8?B?RGx6ZEtLTVNBdElGR3BPY2NuMUF5VVpaVGIvRnlrNk5tRHR0ekxBVmowdUN4?=
 =?utf-8?B?S29teUEyWlByNkRWaFFOM3ZQb2ZHSEg0SC8vN1dDWFlrK2IrMkdmWUk3QmNj?=
 =?utf-8?B?OE9tSGJvTzg3eEpwb0R3cy9kUmJPUkh5eDkwTVkwZkJvcWNEa0JPWStyMWJj?=
 =?utf-8?B?cGJqVm9JSlRnNWtkaEtiUU9GdGlZZkhPU2NYem1ZZDEwYTRrUlYrTzZyanJN?=
 =?utf-8?B?UjB1U00yOGNBRGsrQzcvbmJZYVlHTElKZnBvWGwyL0VKTXA0d3lqdU1BcjRU?=
 =?utf-8?B?VDRBN0J4azBDTzBoeTFZaDhDcjhuSEVReHIwaDZBTjFFUXQ5STE5a1NpZ3hE?=
 =?utf-8?B?UTVaV0t4enI5ZndPRDBMWXRYY2MxTVdjZFlMT0NEbzhjZURTZFpRaFVpNlJa?=
 =?utf-8?B?YU1RMUpXRzk3T2dWMWVOYlg1T3lzaTBhclpnUGFqeGJaZ0NaSU9Vem1COWdZ?=
 =?utf-8?B?YjZyQ2c0UVFUV0lnSXdVaEhlTXQyUDVrU2tTaXdJMkpxR3Z4NGwzeTJwOXpn?=
 =?utf-8?B?SFY1cXBpTEQ0R1JKQkRqeERUeWMvN2dmQ1VXL0oxaGtQVi9JTkh6MU1sMWRG?=
 =?utf-8?B?OTZabmdOT0RUY3JrUDgzd2cwTlJ2RE9Db3docURZazVnU2NZaDVwUmZnZnBl?=
 =?utf-8?B?Um1EanEraWlad3BMcEFKL2hhQ1dpR2Z0N1ExRlJQWkhTaHZ2SlU5OEt4Y2lO?=
 =?utf-8?B?Wm5NQ3UwcHdRWGV4SW9kWFhnbDNxQXlIdEE4U2FReXZaN1hLYnNQYUlHUUMw?=
 =?utf-8?B?SFFRZlJoWThVU3NuMy8vNFl0eWc4dTdWUFhCRjF1dUhyUnJ5RHNza2JaNitC?=
 =?utf-8?B?a3JnK2RXRlBzeHlnbkt6bUlCVWRyNDE1M3JWYnZ4UXlCVUE2K1AzdUE3OUUx?=
 =?utf-8?B?V05pa1ZPTDVIZXBOQkNMU2hWZ3QzVUl3RUw0SnhuY3lHS1E4YnBkM0FDUFF3?=
 =?utf-8?B?d1lkQ2hsZEg1K0JRcVFWY3M4dTdhdS9mbHNHWmRwQWxpOUhIbHY3eDluNE85?=
 =?utf-8?B?UGZ4L0ExMisrVzVmUjRCQ2NNdlh1V1hIZm1TaW1nUStWWGtDQzlzUnpTdlRv?=
 =?utf-8?B?ajVPOXV0d3VjQmQ5Y0M3RGZ4VG4vM1ExM2Z5OU1aRmhXZ1dqSjAwQkJoNGhi?=
 =?utf-8?B?N2hXcm12N1ZJaDlyRFh3bzZQdzB5Z0lNWU0zQ2JCcXVPYXEyNFViOFhOeVVC?=
 =?utf-8?B?Z2pSeWhpdUltNFYyNnhkUHVrSEJTNHNZQ01TV1M1TGhyRlQ1ZC9mY1pjOUt3?=
 =?utf-8?B?a3RoNUMyTTZjZll3TkFFdXpTQ3N6MTU3SzFTWWhEaDY4ck1TeWJqYy8yd0tv?=
 =?utf-8?B?Njhqc24vUGR0RHQ2OUNZalN4c3BMYlF3dTlld0QxR0ZYWXZNbVlRNDNPTFpW?=
 =?utf-8?B?eW05eUoybURJN0NONzQrMUZFK1hvTlZiTFJtU29TRUFYMnNOa09ybWU1UGFv?=
 =?utf-8?B?aWk3SE1IWDNZVFNDOWVWLytsQzZ2V0pqOHh4YVlxa2x1UllDRzMvVU1BWWZ4?=
 =?utf-8?B?OXRpOWtqQnkzbVpTeHVNTmpPUEkxTTZ4YlVMTDBGNlZCUGRPM2MzUmNldWFM?=
 =?utf-8?B?U2o2TGJBWUV6ckFMNXQza1pnUXozbm9WaDFpRGZMZnc0YUlwMjB4c25UbUcw?=
 =?utf-8?B?bitwSjBOVlU4S3BMaHJQZkEvRXFZR05oZ1VkbThkcmUwbWpKdWFvUlRLVzNw?=
 =?utf-8?Q?3QlxlTjb5q1Xbs1e2zEIX6U70?=
X-OriginatorOrg: nuvoton.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 91a52c8d-87ab-4907-2be8-08dc39b9d860
X-MS-Exchange-CrossTenant-AuthSource: SL2PR03MB4348.apcprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Mar 2024 06:35:54.3077
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: a3f24931-d403-4b4a-94f1-7d83ac638e07
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: k6aRrVtpjQwB+XpcwXOrBcQkGsfF/pYghU3RCHYhElCz85mls83kqgObGDxpuv9uyM1VPxU+LD1eij0bXVr3mg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: OS8PR03MB8869


Mark Brown =E6=96=BC 2/22/2024 11:47 PM =E5=AF=AB=E9=81=93:
> On Thu, Feb 22, 2024 at 04:38:25PM +0800, Seven Lee wrote:
>
> A few very minor things below but basically this looks good.
>
>> +static int nau8325_clksrc_choose(struct nau8325 *nau8325,
>> +                             const struct nau8325_srate_attr **srate_ta=
ble,
>> +                             int *n1_sel, int *mult_sel, int *n2_sel)
>> +{
>> +proc_done:
>> +    dev_err(nau8325->dev, "nau8325->fs=3D%d,range=3D0x%x, %s, (n1,mu,n2=
,dmu):(%d,%d,%d), MCLK_SRC=3D%uHz (%d)",
>> +            nau8325->fs, (*srate_table)->range,
>> +            (*srate_table)->max ? "MAX" : "MIN",
>> +            *n1_sel =3D=3D CLK_PROC_BYPASS ?
>> +            CLK_PROC_BYPASS : mclk_n1_div[*n1_sel].param,
>> +            *mult_sel =3D=3D CLK_PROC_BYPASS ?
>> +            CLK_PROC_BYPASS : 1 << mclk_n3_mult[*mult_sel].param,
>> +            1 << mclk_n2_div[*n2_sel].param,
>> +            (*srate_table)->mclk_src[ratio],
>> +            (*srate_table)->mclk_src[ratio] / nau8325->fs);
> This should be a dev_dbg(), dev_err() will be very noisy in normal
> operation.

okay, I will make the correction to dev_dbg().

>
>> +static int nau8325_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
>> +{
>> +    struct snd_soc_component *component =3D dai->component;
>> +    struct nau8325 *nau8325 =3D snd_soc_component_get_drvdata(component=
);
>> +    unsigned int ctrl1_val =3D 0;
>> +
>> +    switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
>> +    case SND_SOC_DAIFMT_CBS_CFS:
>> +            break;
> Please use the modern _CBC_CFC defines.

okay, I will revise.

>
>> +    ret =3D regmap_read(nau8325->regmap, NAU8325_R02_DEVICE_ID, &value)=
;
>> +    if (ret) {
>> +            dev_err(dev, "Failed to read device id (%d)", ret);
>> +            goto err;
>> +    }
> Probably a good idea to check that the device ID is what we expected
> too.

okay, thanks.

________________________________
________________________________
 The privileged confidential information contained in this email is intende=
d for use only by the addressees as indicated by the original sender of thi=
s email. If you are not the addressee indicated in this email or are not re=
sponsible for delivery of the email to such a person, please kindly reply t=
o the sender indicating this fact and delete all copies of it from your com=
puter and network server immediately. Your cooperation is highly appreciate=
d. It is advised that any unauthorized use of confidential information of N=
uvoton is strictly prohibited; and any information in this email irrelevant=
 to the official business of Nuvoton shall be deemed as neither given nor e=
ndorsed by Nuvoton.