linux-kernel - Re: [PATCH 1/2] reset: hisilicon: Add hi6220 media part of reset

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <56CBB9C4.9070900@hisilicon.com>
Date:	Tue, 23 Feb 2016 09:45:40 +0800
From:	Chen Feng <puck.chen@...ilicon.com>
To:	Philipp Zabel <p.zabel@...gutronix.de>
CC:	<arnd@...db.de>, <xinliang.liu@...aro.org>,
	<linux-kernel@...r.kernel.org>, <puck.chen@...mail.com>,
	<w.f@...wei.com>
Subject: Re: [PATCH 1/2] reset: hisilicon: Add hi6220 media part of reset

Hi,

Thanks for review.

On 2016/2/22 18:23, Philipp Zabel wrote:
> Hi Chen,
> 
> Am Montag, den 22.02.2016, 10:45 +0800 schrieb Chen Feng:
>> Signed-off-by: Chen Feng <puck.chen@...ilicon.com>
>> Signed-off-by: Xinliang Liu <xinliang.liu@...aro.org>
>> ---
>>  drivers/reset/hisilicon/hi6220_reset.c | 122 ++++++++++++++++++++++++---------
>>  1 file changed, 90 insertions(+), 32 deletions(-)
>> +/* peritheral ctrl regs
> 
> I assume this is supposed to mean "peripheral" ...
> 
>> + */
>> +#define PERITH_ASSERT_OFFSET      0x300
> 
> ... and this PERIPH_ASSERT_OFFSET, and so on.
> 

Yes...
>> +#define PERITH_DEASSERT_OFFSET    0x304
>> +#define PERITH_MAX_INDEX          0x509

>> +}
> 
> If you make these take the struct reset_controller_dev *rc_dev as first
> parameter, you can drop the two functions below an just add a second set
> of const struct reset_control_ops for the media resets.
> 
okay.
>>  static int hi6220_reset_assert(struct reset_controller_dev *rc_dev,
>>  			       unsigned long idx)
>>  {
>>  	struct hi6220_reset_data *data = to_reset_data(rc_dev);
>> +	struct regmap *regmap = data->regmap;
>>  

>>  	data->rc_dev.ops = &hi6220_reset_ops;
> 
> With media and periph resets in separate ops structures, you could chose
> here already:
> 
> 	if (type == MEDIA)
> 		data->rc_dev.ops = &hi6220_media_reset_ops;
> 	else
> 		data->rc_dev.ops = &hi6220_peripheral_reset_ops;
> 
I will add two ops next version.
>> -	data->rc_dev.of_node = pdev->dev.of_node;
>> +	data->rc_dev.of_node = np;
>> +	if (type == MEDIA)
>> +		data->rc_dev.nr_resets = MEDIA_MAX_INDEX;
>> +	else
>> +		data->rc_dev.nr_resets = PERITH_MAX_INDEX;
>>  
>>  	reset_controller_register(&data->rc_dev);
>>  
>> @@ -89,9 +139,17 @@ static int hi6220_reset_probe(struct platform_device *pdev)
>>  }
>>  
>>  static const struct of_device_id hi6220_reset_match[] = {
>> -	{ .compatible = "hisilicon,hi6220-sysctrl" },
>> -	{ },
>> +	{
>> +		.compatible = "hisilicon,hi6220-sysctrl",
>> +		.data = (void *)PERITHERAL,
>> +	},
>> +	{
>> +		.compatible = "hisilicon,hi6220-mediactrl",
>> +		.data = (void *)MEDIA,
>> +	},
>> +	{ /* sentinel */ },
>>  };
>> +MODULE_DEVICE_TABLE(of, hi6220_reset_match);
>>  
>>  static struct platform_driver hi6220_reset_driver = {
>>  	.probe = hi6220_reset_probe,
> 
> regards
> Philipp
> 
> 
> .
>