From:   Neeraj sanjay kale <neeraj.sanjaykale@nxp.com>
To:     Greg KH <gregkh@linuxfoundation.org>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        "marcel@holtmann.org" <marcel@holtmann.org>,
        "johan.hedberg@gmail.com" <johan.hedberg@gmail.com>,
        "luiz.dentz@gmail.com" <luiz.dentz@gmail.com>,
        "jirislaby@kernel.org" <jirislaby@kernel.org>,
        "alok.a.tiwari@oracle.com" <alok.a.tiwari@oracle.com>,
        "hdanton@sina.com" <hdanton@sina.com>,
        "ilpo.jarvinen@linux.intel.com" <ilpo.jarvinen@linux.intel.com>,
        "leon@kernel.org" <leon@kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
        Amitkumar Karwar <amitkumar.karwar@nxp.com>,
        Rohit Fule <rohit.fule@nxp.com>,
        Sherry Sun <sherry.sun@nxp.com>
Subject: Re: [PATCH v4 3/3] Bluetooth: NXP: Add protocol support for NXP
 Bluetooth chipsets
Thread-Topic: [PATCH v4 3/3] Bluetooth: NXP: Add protocol support for NXP
 Bluetooth chipsets
Thread-Index: AQHZR3NJNMeQaxHX80659kDSqon6Tg==
Date:   Thu, 23 Feb 2023 10:40:45 +0000
Message-ID: <AM9PR04MB8603910A3247C4649648C21DE7AB9@AM9PR04MB8603.eurprd04.prod.outlook.com>
References: <20230221162541.3039992-1-neeraj.sanjaykale@nxp.com>
 <20230221162541.3039992-4-neeraj.sanjaykale@nxp.com>
 <Y/T1uMqUeW67tgzX@kroah.com>
In-Reply-To: <Y/T1uMqUeW67tgzX@kroah.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?mdPVU4w7H+tsByCbpI1CxU75aPOJ56AlOt+GhHfoTee3zHRpb7zr+vgwXhdf?=
 =?us-ascii?Q?LrAoSrZYkFe4g+dWBOvq0a8bq1XdGLscUC/twIEgFjlxN3hdrHciJ26KCb6a?=
 =?us-ascii?Q?LKIas3BZer1topeNQErExFg+v0jdxrDLfblLfFO3xzn0pQ46ZdlniR6ZkP8S?=
 =?us-ascii?Q?6gTtEtVmQvK4odJkEbT5i1A40x7ZKyDVWRyv6gtxNNL0OqbbH/a6N/t0R8EK?=
 =?us-ascii?Q?5GVn+ArKjdLq1WYUVUCJNezmmXe5k04mwZsNcVp9xsJlPQAW24cAhiGYqofE?=
 =?us-ascii?Q?pVqjIxcvwv0ZFR69wiL+2mUZ8XRTjS3WM4NyPKOKaf1I0XRJKJi2A2qmZcu/?=
 =?us-ascii?Q?TkPXEU+WQ2iEzSuPHhEKfj79xmZDmER5LezRc885ANp2UxrlghESZhkI2Cbx?=
 =?us-ascii?Q?DQM8lWhyDY5lwKSk25kQLtmZZ4n9VfxwxL8bOgQc88xFXbpYNUN4KYn3uAxS?=
 =?us-ascii?Q?n2GPsSAKCGikJKC3Ck7j8G4wkeY4IFjFD30qQoSci/ir030BeH+YX6D1TErY?=
 =?us-ascii?Q?13rt/sb7kKRdNHdFsPQDZcTrek9CyhUc6WPBsIlpMxoGnEi1DBnUnGn9Ncx9?=
 =?us-ascii?Q?o4xOCGbNmH95pt4E/ijOqjvS143IDPhQz0RNTkDjQuy3D3ROKNDSFrJ120XX?=
 =?us-ascii?Q?bA6f/hCAvJUhpvWnVHi0ACYajarkmDuaWgEMwHRTMluXfF5RFUsUbUYyi/yK?=
 =?us-ascii?Q?5eYOZCzNcD4kCehsD3EnJareivdGVVnn3XLU94mrf+rSl6/f2MW0ysiOUJmy?=
 =?us-ascii?Q?AoK70rkw84s0JOtcYUbColKtEHPG/u/1ed4WmBqyY4OTIQFZdXSkUm0S4SnA?=
 =?us-ascii?Q?h6lQAU2mmqMq4Sbh9ljO6K9wK678Fxrfmc3z0gcLiBOHYGJo9AS/GJAtdV+w?=
 =?us-ascii?Q?iYkOv7jAusA8qffFw1sBrMO6t+ne9mGYXkZ1hG8wKJ7O5kxlBBOCukU/tns+?=
 =?us-ascii?Q?vs+l6JxaedquoeDE3oCxqHDsYtIrLRzDBAGTfM/sBWyjPElJpIH3aqMUYg7A?=
 =?us-ascii?Q?ZzMN+LxHTi+Af1AcOzxc0C5N3XCy/VQ6IVJ12t4f14E7oOo2Gc4ZuQMoOa+4?=
 =?us-ascii?Q?Pag8ZhHue10AgP6VUPV83uUjp327L0AbrbK+zzLryrYX4coTMtTmMeowADJA?=
 =?us-ascii?Q?FUvD4w6SH0hs0iCn0xUh1X4rFp2p8buEgyQzQgCXq+1fRg+eWF4ZCExLwfWf?=
 =?us-ascii?Q?pF78whPb07AbrzaxM0kxXIfzl5OJwYEUQLNFTjbVo6+kDZsfBl379dtQ/0Gj?=
 =?us-ascii?Q?ltfXN2GCirUCSfrtADwSE5sX+pNkQM/K69megTT7FN2cqhg3Mf741tEZszQB?=
 =?us-ascii?Q?+9N7wUGr+kyFEdpXJKRS7QEKlC77AHGlB+XY+YcyQ/Sbyc9ZnAz0D0s2Jt+g?=
 =?us-ascii?Q?H1ihS13uxN6SyV0UnBftsuVt9m9neEwO7cH0/4gjppA2w76Dkg4sbb9YjGSp?=
 =?us-ascii?Q?OEIxi2lSbyYlWmmEQwE53wkjdPnAOa4ddw8J97HIW6Gk0arw92rIBx0EsGee?=
 =?us-ascii?Q?3qdIQoWGAa835ZQNZ053oIl0jW57O1M+j2viTk+k7XJ+XY5aIiuvR96ViJcX?=
 =?us-ascii?Q?HTGfo9Tkge+SUdu00B3Yv0VNMOwizsfiYZI6wCGnpAuhPEa+XQKf6QuOHJBc?=
 =?us-ascii?Q?Eg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: AM9PR04MB8603.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 37651d70-2489-48c6-5461-08db158a6bb2
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Feb 2023 10:40:45.9263
 (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: 2AWy27pIhYhbT6rGjP4JvcG/AKpaB7kthGOzkERJr9T1v01y0yD3ZLnCGuCcGYTwCV5P1ayxcOzcoBQNbKDlcoC6cTEohMOwaXD2lQxi2OY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR04MB6945
Precedence: bulk
List-ID: <linux-bluetooth.vger.kernel.org>
X-Mailing-List: linux-bluetooth@vger.kernel.org

Hi Greg,

Thank you for reviewing this patch.

> > +             bt_dev_info(hdev, "Set UART break: %s, status=3D%d",
> > +                         ps_state =3D=3D PS_STATE_AWAKE ? "off" : "on"=
,
> > + status);
>=20
> You have a lot of "noise" in this driver, remove all "info" messages, as =
if a
> driver is working properly, it is quiet.
>=20
Replaced all bt_dev_info() and bt_dev_err() with bt_dev_dbg() for all insta=
nces where user action is not possible.

>=20
> > +     } else if (req_len =3D=3D sizeof(uart_config)) {
> > +             uart_config.clkdiv.address =3D __cpu_to_le32(CLKDIVADDR);
> > +             uart_config.clkdiv.value =3D __cpu_to_le32(0x00c00000);
> > +             uart_config.uartdiv.address =3D __cpu_to_le32(UARTDIVADDR=
);
> > +             uart_config.uartdiv.value =3D __cpu_to_le32(1);
> > +             uart_config.mcr.address =3D __cpu_to_le32(UARTMCRADDR);
> > +             uart_config.mcr.value =3D __cpu_to_le32(MCR);
> > +             uart_config.re_init.address =3D __cpu_to_le32(UARTREINITA=
DDR);
> > +             uart_config.re_init.value =3D __cpu_to_le32(INIT);
> > +             uart_config.icr.address =3D __cpu_to_le32(UARTICRADDR);
> > +             uart_config.icr.value =3D __cpu_to_le32(ICR);
> > +             uart_config.fcr.address =3D __cpu_to_le32(UARTFCRADDR);
> > +             uart_config.fcr.value =3D __cpu_to_le32(FCR);
> > +             uart_config.crc =3D swab32(nxp_fw_dnld_update_crc(0UL,
> > +                                                             (char *)&=
uart_config,
> > +                                                             sizeof(ua=
rt_config) - 4));
> > +             serdev_device_write_buf(nxpdev->serdev, (u8 *)&uart_confi=
g,
> req_len);
> > +             serdev_device_wait_until_sent(nxpdev->serdev, 0);
>=20
> You are sending magic commands over the serial connection, are you sure
> that is ok?
Yes, we are sending this only when the BT chip's bootloader is requesting f=
or payload for the CMD5 sent earlier during FW download.

Thanks,
Neeraj