lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-ID:
 <MEYP282MB2697C5EE3F941651ED59AA5FBB692@MEYP282MB2697.AUSP282.PROD.OUTLOOK.COM>
Date: Wed, 10 Jan 2024 07:15:48 +0000
From: Jinjian Song <SongJinJian@...mail.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"chandrashekar.devegowda@...el.com" <chandrashekar.devegowda@...el.com>,
	"chiranjeevi.rapolu@...ux.intel.com" <chiranjeevi.rapolu@...ux.intel.com>,
	"haijun.liu@...iatek.com" <haijun.liu@...iatek.com>,
	"m.chetan.kumar@...ux.intel.com" <m.chetan.kumar@...ux.intel.com>,
	"ricardo.martinez@...ux.intel.com" <ricardo.martinez@...ux.intel.com>,
	"loic.poulain@...aro.org" <loic.poulain@...aro.org>, "ryazanov.s.a@...il.com"
	<ryazanov.s.a@...il.com>, "johannes@...solutions.net"
	<johannes@...solutions.net>, "davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "pabeni@...hat.com"
	<pabeni@...hat.com>, "linux-kernel@...r.kernel.com"
	<linux-kernel@...r.kernel.com>, "vsankar@...ovo.com" <vsankar@...ovo.com>,
	"danielwinkler@...gle.com" <danielwinkler@...gle.com>, "nmarupaka@...gle.com"
	<nmarupaka@...gle.com>, "joey.zhao@...ocom.com" <joey.zhao@...ocom.com>,
	"liuqf@...ocom.com" <liuqf@...ocom.com>, "felix.yan@...ocom.com"
	<felix.yan@...ocom.com>, Jinjian Song <jinjian.song@...ocom.com>
Subject: Re: [net-next v3 2/3] net: wwan: t7xx: Add sysfs attribute for device
 state machine


>On Thu, 28 Dec 2023 17:44:10 +0800 Jinjian Song wrote:
>> From: Jinjian Song <jinjian.song@...ocom.com>
>> 
>> Add support for userspace to get the device mode, e.g., 
>> reset/ready/fastboot mode.

>We need some info under Documentation/ describing the file / attr and how it's supposed to be used.

Yes, please let me add to the t7xx.rst document

>> diff --git a/drivers/net/wwan/t7xx/t7xx_modem_ops.c 
>> b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
>> index 24e7d491468e..ae4578905a8d 100644
>> --- a/drivers/net/wwan/t7xx/t7xx_modem_ops.c
>> +++ b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
>> @@ -192,6 +192,7 @@ static irqreturn_t t7xx_rgu_isr_thread(int irq, 
>> void *data)  {
>>  	struct t7xx_pci_dev *t7xx_dev = data;
>>  
>> +	atomic_set(&t7xx_dev->mode, T7XX_RESET);

>Why is ->mode atomic? There seem to be no RMW operations on it.
>You can use READ_ONCE() / WRITE_ONCE() on a normal integer.

Please let me modify as suggested.

>>  	msleep(RGU_RESET_DELAY_MS);
>>  	t7xx_reset_device_via_pmic(t7xx_dev);
>>  	return IRQ_HANDLED;
>> diff --git a/drivers/net/wwan/t7xx/t7xx_pci.c 
>> b/drivers/net/wwan/t7xx/t7xx_pci.c
>> index 91256e005b84..d5f6a8638aba 100644
>> --- a/drivers/net/wwan/t7xx/t7xx_pci.c
>> +++ b/drivers/net/wwan/t7xx/t7xx_pci.c
>> @@ -52,6 +52,68 @@
>>  #define PM_RESOURCE_POLL_TIMEOUT_US	10000
>>  #define PM_RESOURCE_POLL_STEP_US	100
>>  
>> +static ssize_t t7xx_mode_store(struct device *dev,
>> +			       struct device_attribute *attr,
>> +			       const char *buf, size_t count) {
>> +	struct pci_dev *pdev;
>> +	struct t7xx_pci_dev *t7xx_dev;
>> +
>> +	pdev = to_pci_dev(dev);
>> +	t7xx_dev = pci_get_drvdata(pdev);
>> +	if (!t7xx_dev)
>> +		return -ENODEV;
>> +
>> +	atomic_set(&t7xx_dev->mode, T7XX_FASTBOOT_DL_SWITCHING);

>This function doesn't use @buf at all. So whenever user does write() to the file we set to SWITCHING? Shouldn't we narrow down the set of accepted values to be able to add more functionality later?

Yes, it doesn't use buff, just now only allows user setting SWITCHING.
Please let me narrow down the set to be more functionality.

>> +	return count;
>> +};

>unnecessary semi-colon

Best Regards,
Jinjian

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ