Message-ID: <ae765e9d-b3dc-0047-f080-e2d059b3821a@intel.com>
Date:   Mon, 10 Jan 2022 19:58:01 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.3.0
Subject: Re: [PATCH 1/5] ASoC: sh: rz-ssi: Drop calling rz_ssi_pio_recv()
 recursively
Content-Language: en-US
To:     Pavel Machek <pavel@denx.de>,
        "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
CC:     Biju Das <biju.das.jz@bp.renesas.com>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
        alsa-devel <alsa-devel@alsa-project.org>,
        LKML <linux-kernel@vger.kernel.org>
References: <20220110094711.8574-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <20220110094711.8574-2-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <2f338a37-f2ca-33e4-284e-5d263f7b93da@intel.com>
 <CA+V-a8vz25B=cw_C4YMBRdDxeq7mi8Zc+noqpdHqfMP8eNHYFg@mail.gmail.com>
 <20220110184418.GE3396@duo.ucw.cz>
From:   Cezary Rojewski <cezary.rojewski@intel.com>
In-Reply-To: <20220110184418.GE3396@duo.ucw.cz>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?V0NLYWR5bjZUbHhNU3EyVTgyZ3V0YWdnTWU4Ui81a29jRW9RMkE1V0U5cVNE?=
 =?utf-8?B?SkJkL2RsVjk3OGJTYWRYY251Z3JiMFpRMGo5Mkp3U25yUHI2NS84ekc1TXhh?=
 =?utf-8?B?QWtRNGpxK0FsMDh2RndnUVV3alhITlg2K09UZU1lZWtTa1JQS2FFaEx0Y0dL?=
 =?utf-8?B?REpQYXIyTjNaOEhDT1RsTlM2NEh3V1Y0azRvUTNhWjdndThkOW5wODYwc0RM?=
 =?utf-8?B?b2xMcWpubDVhODR5dUV1M3Q0bWxzS2NtU0kwRE5PYkZmQmxmSkd2YmhoQnRy?=
 =?utf-8?B?QWo5WmVYRnVtdUQzNEFrOHFwZG51aHBpbmZWbFBqeUV3aHVpUFBIUlFYMVlJ?=
 =?utf-8?B?UVQ4WWdoc0FjY3JoaE9JY2F0MEpXQ0M3S2lTUk9Ia0pnMHliVnFhYnNwNUE0?=
 =?utf-8?B?eWtPViszdngvQ0YrZTRvb3lSOTNDNTd2N0cwYXlPSEk1cjZuVVFXRVFoRFk3?=
 =?utf-8?B?bXVmeDlJR3V4ZXNHd2c5Z3NDOWw5cUtrTXAyNEY3Mzg5Vnl1S0wrMFV3UG0x?=
 =?utf-8?B?bjVUc3NmUUwxWWQwekZmeVBSYmFUWHhPbHl1MmJna3hRQVVMWkhCQWp2cmJn?=
 =?utf-8?B?YnF2TkJ1ekhMU0JrRXZFNzMrNWZxVm90REx2OVFNWGEyUkh5VkxIdGpua3My?=
 =?utf-8?B?blRtZnZ1RU5FcFZzOXkrVjQ5MHB5MzVybUtVRjJBQXNKZzIvaE1mMVJoUW5Z?=
 =?utf-8?B?OUxnNFo2M3BDVzNUV0hhb0JNRjQ5SGhQeVlTZ2hTTzVGT01RdHlycnQyM296?=
 =?utf-8?B?Zm56Q21HYk1iZDZpQnQ2d2FKTWpEUXUxaWRNVnVvb1F4RUVXSVY2ZmNkM1V2?=
 =?utf-8?B?bEJveVBkUm5BUlM2MUR2bzhzQlJkYzU0MGoxY3BEOENOb0JLeElaQjIwbU5q?=
 =?utf-8?B?d2hXd3lTOENDNnJZdVl5dTZheEl0cjZDSUxHTE5DSWttcklXcGlXWnZUbU0y?=
 =?utf-8?B?VGVJNDhvOEFBd2VWNjlVSnVhYndIOEhSa051VlJBMFIxZGhsVThwbytlemVO?=
 =?utf-8?B?aWVYellPNzBlekxDR05aT2Z4MElEVjNEQnBxTTYwbjlGV25ZY1hyMVkrSklh?=
 =?utf-8?B?cmVoak52MjVoWlliZlRKaGZhYm9MclplYmdBbmtXQVRRTWMwRlRiTEpvVGlT?=
 =?utf-8?B?OGRrRzIxc0FscjdONnhBQkdqL0VUdDNoTFlGVklVTWQ3Y3BLdmVwTy9ZVW9l?=
 =?utf-8?B?ZmNDc1l2aVRodmU3ZFBWcFpuTEFZOXVGZ0ZsUnZRVXhNNXRlWlNBWDVkQXov?=
 =?utf-8?B?ZlFISHk2SzJVOTFEYm9JQmZ4NG1VQjNVRVVNMDVJTWkrRzVjODl6eW1PM2E1?=
 =?utf-8?B?UHlDZFZkOHlORk9DYmY3YUkvRVk1a3g2S2NYK0doZUN0TlJtUS9GMmh0Y09t?=
 =?utf-8?B?SXFoRjRqWUtvaEdocjVPUGdDUC82UXZrOVp2bXlURXdXVW44bjhPL21PUUJv?=
 =?utf-8?B?bTFyNUpScHRJdGQxdUNoeTJnRndZa1NjL1pGa3NLS2VNUFVFaGgyYmt6VjJH?=
 =?utf-8?B?bUVZSC8xY0l0Y3o4dWs4akcyR3dZd1dIcWhPZDgvS3h2d0Z6eEhad2FwbmlT?=
 =?utf-8?B?blVoYVlKcW9MenhHMkp4Z05hTEhhYUtXZEFpcTBHR0N2eVkva09DTlRPQ1M2?=
 =?utf-8?B?cFJid0NabE80NkUzWSsvdzV2NHgvcjViTVl2cklTb01IQXpKN0xBRmF5aGd2?=
 =?utf-8?B?cEUyNEpGT2ZwVCtvQ0JXWFRFYTkySWdMSHp5eGRUUUx3STZUZk1idndFbW4y?=
 =?utf-8?B?anFjbkd1MnliaXVINUF6ZlBucW10SlE4VndIQU5WTTVmNy83c3pjdHhLc01z?=
 =?utf-8?B?S2cvdkdSTXNhb25VTk5GL2U0Wk14KzlVN3V5ZDUxUUxyamNDVVhBRUNoWDVH?=
 =?utf-8?B?M3RqeDBoYUpRWi9GQkNNN2VrSStuN3hTWldteWx5OGNyUkJ2bm9valpmN3Uy?=
 =?utf-8?B?WGNIenJqUHc4RjVJOGFpOEVjRFVIWUM5MlA5WXFCNktoaTJVQWZUblR2S3ZI?=
 =?utf-8?B?NU1Ydk4vUUpyYkl2YzFFSTM3ZXA3OEZjZm1YbzhRMVVibS93ZVJJYks1U3Ez?=
 =?utf-8?B?RWNPNjZtSHl2YWlEN3hMUVJxd1hKSlFLQlFvMTNLMEFXbkVvbDB1SXJkcThD?=
 =?utf-8?B?VUtnSDdON1cxbFNzcnJqVm9IQnRXQy9qOGI3Z0J6SjNMWW1zbXpkVDNvZUMr?=
 =?utf-8?B?ZjlKS2E1V0xITFQrNTZvOVdpNkNpeVV0djFKbXRGbFUyeXRreVhwQTdLVEtw?=
 =?utf-8?B?RW1YUjg0NFUrNmJhOEUxSGw5OFh3PT0=?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 7a5d3c97-8bb4-46ce-de31-08d9d46b2387
X-MS-Exchange-CrossTenant-AuthSource: BN6PR11MB4049.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Jan 2022 18:58:07.5430
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 3uM5PiTMlyURWM5HcjTe3FgJqAub0q13AO/k03noaAOgj1QDI/K6Bqv6gtgvphiRvxx9BZpHEmOjKhA0BUhljq5xXjfPm0Q+JWDcjfqYiwA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR11MB2657
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2022-01-10 7:44 PM, Pavel Machek wrote:
> Hi!
> 
>>> On 2022-01-10 10:47 AM, Lad Prabhakar wrote:
>>>> Instead of recursively calling rz_ssi_pio_recv() use a while loop
>>>> to read the samples from RX fifo.
>>>
>>> Recursion and loops are means for doing something repeatedly. Could you
>>> specify _why_ such change was made i.e. the conversion from one method
>>> into the other? I bet the code is not being changed for the sake of
>>> changing it, the reason is simply missing in the commit message.
>>>
>> I had feedback from Pavel "recursion is unwelcome in kernel due to
>> limited stack use." which I did agree with as a result I have come up
>> with this patch. Also to add this driver will later be used on Renesas
>> RZ/A2 SoC's which runs with limited memory.

...

> 
> Yes, loop is better.
> 
> I'd actually do while(true) and avoid using the done variable.
> 
>      if (!(!frames_left && fifo_samples >= runtime->channels))
>                break;
> 
> will do the trick. Better yet, do
> 
>      if (frames_left || fifo_samples < runtime->channels)
>                break;
> 
> because double negation is quite confusing and looks like typo.

You could achieve similar results by enlisting do-while loop. That's my 
proposal.


Regards,
Czarek