Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 31 Jan 2022 16:45:58 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
Cc:     gregkh@linuxfoundation.org, realwakka@gmail.com,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] staging: pi433: add debugfs interface
Message-ID: <20220131134558.GL1951@kadam>
References: <20220126132116.GA1951@kadam>
 <YfX+llwDWZZMz+NY@mail.google.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YfX+llwDWZZMz+NY@mail.google.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?3RYUs3Amr824Cy8V78/YPiwjB+WFfgSS2xGxsfGw48ukERb53N6s2Qv0neMB?=
 =?us-ascii?Q?slZqnM3Npnl28ggJOq4l+eT0YM9i587VKy/QU7BLs4jbCpDP9SLntJTAjl1v?=
 =?us-ascii?Q?phMWxEXc6rDLb5WUrBbVWU2X/DayohWzOY8aa/fi5EHCE+S1odJHJepHHXT0?=
 =?us-ascii?Q?y20ktj77AqBmwXcfGGw+YsYU/KgFoBXaPD01yibdBRCzM6DdgvC4XWaIpSUa?=
 =?us-ascii?Q?DCcwDhFlq/1HB0bphUZN0hVp78e3mzZEovfjqoSPtiqd6+UT83i+Qnpm4Jqh?=
 =?us-ascii?Q?LUGDSoopsQNzFsTUGE8nQTPNn2rdYjT2uXHwXl+0O7QAls2MoaMkbLmRyg7Q?=
 =?us-ascii?Q?mKZiI73fYErrgBdvSM47AhDKZRMjG+xNzm0zlUk0wPs8qFlVzmkdEn4bYULg?=
 =?us-ascii?Q?SuJ/xdwCtdKdH1OzwJ6PDcm17r7PNeUT2kxx0e2le7pDHS/mAZ/JJlM3+QMx?=
 =?us-ascii?Q?nYyoaLNPmR86NRDYb5NBvFmRbrkWEaQ4N27L5nij/85JgeHyX4FyMYgSMkBG?=
 =?us-ascii?Q?xkxSU9I+YKnmFEoDhogkegAGV5CLFYZMLaQmgg0+CHLFVA+9CxIAA692tjSb?=
 =?us-ascii?Q?kkS5/CaxHypr1oUHwtNmJJXxXFqsdWlWCI0IWSivx2uuTkRzMhFxTjA70kDT?=
 =?us-ascii?Q?GUdKe2wwDeHbilNwSSsJ0cScwh3fDgRkC1+bJ3SXYi1zHPe4d/A/5xzYWkbe?=
 =?us-ascii?Q?cB7YuihDxN87hx5NO7oQ5F+6Gyn70Xv+rKv44TOWzKqRzwnT5l+yRKwbTvon?=
 =?us-ascii?Q?ged28Gr7tVBOM9H/LsC08n9GlZDYo0WB0q6xMCz2I5OHYE793tzVbCswue9o?=
 =?us-ascii?Q?JQOlmyMjq2RU0jXbla/7e/ndRTcBbtFGNA1Qiw1QtNPjIx1VgOKKvv3P2J8F?=
 =?us-ascii?Q?sa3WtPGakSTM8X8/Qsk50gFNaNGzWWvx1Iz/NIx1Aol5D/7LyIW9MhKNIqi1?=
 =?us-ascii?Q?g5OAw7M1+1kfi5edLYTg0Dj280dg8N5lWYhF1Zes7n8fqGtizvHl+J89uFEK?=
 =?us-ascii?Q?AeLae9JNvK0ASl5xmAiA8y/2LS1b/R60vgGfMzTXp9DvOYh9T05VM7TMSrxj?=
 =?us-ascii?Q?1SWg3L1KOxknwZEi/oJIoYOhI7LrZYDSb0QgPpcNnOgWST97ueWIhI1aMmtO?=
 =?us-ascii?Q?Z1lZNw0UovhOCTwpMhmDzjNAVqbgsyCYAai1ftCVSN88fZzI6q3yIlVu9wnU?=
 =?us-ascii?Q?izwGnuQvLioqaenR7wfZrOWzEDZ910241ZvADOk7KT+cLrVyW3yfwpeEozGf?=
 =?us-ascii?Q?H4mY9INNIRbIu+G5e7tXDm9Yi2FqdQTPQt4sZEWfSpdCR2x/jwf1PC+VNN5w?=
 =?us-ascii?Q?yMVKFasQlB6nmi+hM5EULc78WHVyIr5agUaZkTgtbbjTUw029NFOvTEfmyVy?=
 =?us-ascii?Q?omINAsVPN6kI6cNSqXWO3K3Zx0EInVI08aVEiC4UXG+1m44k0L+IeElhWGIZ?=
 =?us-ascii?Q?OXUZ/Yrs0JPRwYNq6MC+QrNQTAhgdStC/m7w7oaBLkCqbj0TOiPW77pC6uyT?=
 =?us-ascii?Q?fA4Z7M3pR2WwOpX1DbmS8yE/dxXdUbqtA3fRxNyoLVwBwMx52bRHG3BGhf3V?=
 =?us-ascii?Q?QRrgm3YBXy0yZMUq2ZzdPohHblbV1AkEnEHLvWQpbFm6pONnRI/ThyklYPVO?=
 =?us-ascii?Q?j+2E/eXR7PHvY2qLRgk18MHSYdBj80oxj6jtP6pBxQacQNQY4UX9ZRVkgA2P?=
 =?us-ascii?Q?pqGsqaIx9K4FnZ3VPltueUUbdEM=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 317a8576-fb57-42f0-7ec2-08d9e4c00ebb
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2022 13:46:18.3220
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: LOzZQoLxHnLJzCkciEErOE8PGl8pT+P9EuOL5C6kzmNVmpQAOsx6v0eChRN0eY4yyHdKTeRO8ieBz1UoU1okAxFRjt+DyljC+74T3BNOCq8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR10MB3441
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10243 signatures=673430
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 malwarescore=0 phishscore=0
 mlxscore=0 adultscore=0 suspectscore=0 mlxlogscore=999 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000
 definitions=main-2201310092
X-Proofpoint-ORIG-GUID: T_TLVqZNoky3m3s9r5mHOx8L21DNLI-6
X-Proofpoint-GUID: T_TLVqZNoky3m3s9r5mHOx8L21DNLI-6
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sun, Jan 30, 2022 at 03:57:26PM +1300, Paulo Miguel Almeida wrote:
> @@ -1102,12 +1104,75 @@ static const struct file_operations pi433_fops = {
>  	.llseek =	no_llseek,
>  };
>  
> +static int pi433_debugfs_regs_show(struct seq_file *m, void *p)
> +{
> +	struct pi433_device *dev;
> +	u8 reg_data[114];
> +	int i;
> +	char *fmt = "0x%02x, 0x%02x\n";
> +	int ret = 0;

No need to initialize.  Bogus initializers just disable ten thousand
person hours spent developing static analysis.

> +
> +	dev = m->private;
> +
> +	mutex_lock(&dev->tx_fifo_lock);
> +	mutex_lock(&dev->rx_lock);
> +
> +	// wait for on-going operations to finish
> +	ret = wait_event_interruptible(dev->rx_wait_queue, !dev->tx_active);
> +	if (ret)
> +		return ret;

Drop the two mutexes before returning.

> +
> +	ret = wait_event_interruptible(dev->tx_wait_queue, !dev->rx_active);
> +	if (ret)
> +		return ret;

Drop the mutexes.

> +
> +	// skip FIFO register (0x0) otherwise this can affect some of uC ops
> +	for (i = 1; i < 0x50; i++)
> +		reg_data[i] = rf69_read_reg(dev->spi, i);
> +
> +	reg_data[REG_TESTLNA] = rf69_read_reg(dev->spi, REG_TESTLNA);
> +	reg_data[REG_TESTPA1] = rf69_read_reg(dev->spi, REG_TESTPA1);
> +	reg_data[REG_TESTPA2] = rf69_read_reg(dev->spi, REG_TESTPA2);
> +	reg_data[REG_TESTDAGC] = rf69_read_reg(dev->spi, REG_TESTDAGC);
> +	reg_data[REG_TESTAFC] = rf69_read_reg(dev->spi, REG_TESTAFC);
> +
> +	seq_puts(m, "# reg, val\n");
> +
> +	for (i = 1; i < 0x50; i++)
> +		seq_printf(m, fmt, i, reg_data[i]);
> +
> +	seq_printf(m, fmt, REG_TESTLNA, reg_data[REG_TESTLNA]);
> +	seq_printf(m, fmt, REG_TESTPA1, reg_data[REG_TESTPA1]);
> +	seq_printf(m, fmt, REG_TESTPA2, reg_data[REG_TESTPA2]);
> +	seq_printf(m, fmt, REG_TESTDAGC, reg_data[REG_TESTDAGC]);
> +	seq_printf(m, fmt, REG_TESTAFC, reg_data[REG_TESTAFC]);
> +
> +	mutex_unlock(&dev->rx_lock);
> +	mutex_unlock(&dev->tx_fifo_lock);
> +
> +	return ret;
> +}

regards,
dan carpenter