linux-kernel - Re: [PATCH v3 2/2] gpio: loongson: add firmware offset parse support

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>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <04c72d20-6f4d-b957-ee77-6bcbe279f8b2@loongson.cn>
Date:   Wed, 23 Aug 2023 15:36:32 +0800
From:   Yinbo Zhu <zhuyinbo@...ngson.cn>
To:     andy.shevchenko@...il.com
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>, linux-gpio@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jianmin Lv <lvjianmin@...ngson.cn>, wanghongliang@...ngson.cn,
        loongson-kernel@...ts.loongnix.cn, zhuyinbo@...ngson.cn
Subject: Re: [PATCH v3 2/2] gpio: loongson: add firmware offset parse support


Hi andy,

Sorry, I lost this email due to issues with my company's email server. I
will adopt your suggestion in v5 version.

在 2023/8/15 上午5:48, andy.shevchenko@...il.com 写道:
> Mon, Aug 07, 2023 at 03:40:43PM +0800, Yinbo Zhu kirjoitti:
>> Loongson GPIO controllers come in multiple variants that are compatible
>> except for certain register offset values.  Add support for device
>> properties allowing to specify them in ACPI or DT.
> 
>> +	if (device_property_read_u32(dev, "ngpios", &ngpios) || !ngpios)
>> +		return -EINVAL;
>> +
>> +	ret = DIV_ROUND_UP(ngpios, 8);
>> +	switch (ret) {
>> +	case 1 ... 2:
>> +		io_width = ret;
>> +		break;
>> +	case 3 ... 4:
>> +		io_width = 0x4;
>> +		break;
>> +	case 5 ... 8:
>> +		io_width = 0x8;
>> +		break;
>> +	default:
>> +		dev_err(dev, "unsupported io width\n");
>> +		return -EINVAL;
>> +	}
> 
> Why? We have bgpio_init() handle this.
> https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git/commit/?h=gpio/for-next&id=55b2395e4e92adc492c6b30ac109eb78250dcd9d


okay, I got it.

> 
> ...
> 
>> +	lgpio->chip.can_sleep = 0;
> 
> It's boolean, use boolean initializer.


okay, I got it.

> 
> ...
> 
>> +	if (lgpio->chip_data->label)
>> +		lgpio->chip.label = lgpio->chip_data->label;
>> +	else
>> +		lgpio->chip.label = kstrdup(to_platform_device(dev)->name, GFP_KERNEL);
> 
> No error check? Not a devm_*() variant, so leaking memory?


This code had been removed in v4.

> 
> ...
> 
>> +	{
>> +		.id = "LOON0007",
>> +	},
> 
> How does DSDT excerpt for this device look like?


LOON0007 and LOON000A are similar, LOON000A is for 2k2000 gpio0.

      Device (GPO0)
         {
             Name (_HID, "LOON000A")  // _HID: Hardware ID
             Name (_ADR, Zero)  // _ADR: Address
             Name (_UID, One)  // _UID: Unique ID
             Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource 
Settings
             {
                 QWordMemory (ResourceConsumer, PosDecode, MinFixed, 
MaxFixed, NonCacheable, ReadWrite,
                     0x0000000000000000, // Granularity
                     0x000000001FE00500, // Range Minimum
                     0x000000001FE00520, // Range Maximum
                     0x0000000000000000, // Translation Offset
                     0x0000000000000021, // Length
                     ,, , AddressRangeMemory, TypeStatic)
             })
             Name (_DSD, Package (0x02)  // _DSD: Device-Specific Data
             {
                 ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* 
Device Properties for _DSD */,
                 Package (0x01)
                 {
                     Package (0x02)
                     {
                         "ngpios",
                         0x20
                     }
                 }
             })
         }


Thanks,
Yinbo