linux-kernel - Re: [PATCH v2] spi/pl022: add devicetree 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]

Date:	Tue, 26 Jun 2012 10:49:27 -0300
From:	Alexandre Pereira da Silva <aletes.xgr@...il.com>
To:	Thierry Reding <thierry.reding@...onic-design.de>
Cc:	Roland Stigge <stigge@...com.de>, linux-doc@...r.kernel.org,
	devicetree-discuss@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
	Rob Herring <rob.herring@...xeda.com>,
	Rob Landley <rob@...dley.net>,
	spi-devel-general@...ts.sourceforge.net
Subject: Re: [PATCH v2] spi/pl022: add devicetree support

On Tue, Jun 26, 2012 at 3:09 AM, Thierry Reding
<thierry.reding@...onic-design.de> wrote:
> On Mon, Jun 25, 2012 at 06:06:28PM -0300, Alexandre Pereira da Silva wrote:
> [...]
>> diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
> [...]
>> @@ -1733,7 +1744,7 @@ static int calculate_effective_freq(struct pl022 *pl022, int freq, struct
>>   * A piece of default chip info unless the platform
>>   * supplies it.
>>   */
>> -static const struct pl022_config_chip pl022_default_chip_info = {
>> +static struct pl022_config_chip pl022_default_chip_info = {
>>       .com_mode = POLLING_TRANSFER,
>>       .iface = SSP_INTERFACE_MOTOROLA_SPI,
>>       .hierarchy = SSP_SLAVE,
>
> What's the reason for removing the const? You're only reading data from
> the structure.

I are right. I will remove this change. It was a left-over from an
older version.

> [...]
>> @@ -1998,8 +2039,14 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
>>               goto err_no_pdata;
>>       }
>>
>> +     num_cs = platform_info->num_chipselect;
>> +#ifdef CONFIG_OF
>> +     of_property_read_u32(np, "pl022,num-chipselects", &num_cs);
>> +#endif
>
> This...
>
> [...]
>> @@ -2017,13 +2064,40 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
>>        * on this board
>>        */
>>       master->bus_num = platform_info->bus_id;
>> -     master->num_chipselect = platform_info->num_chipselect;
>> +     master->num_chipselect = num_cs;
>>       master->cleanup = pl022_cleanup;
>>       master->setup = pl022_setup;
>>       master->prepare_transfer_hardware = pl022_prepare_transfer_hardware;
>>       master->transfer_one_message = pl022_transfer_one_message;
>>       master->unprepare_transfer_hardware = pl022_unprepare_transfer_hardware;
>>       master->rt = platform_info->rt;
>> +     master->dev.of_node = dev->of_node;
>> +
>> +#ifdef CONFIG_OF
>> +     for (i = 0; i < num_cs; i++) {
>> +             int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
>> +
>> +             if (cs_gpio == -EPROBE_DEFER) {
>> +                     status = -EPROBE_DEFER;
>> +                     goto err_no_gpio;
>> +             }
>> +
>> +             pl022->chipselect[i] = cs_gpio;
>> +
>> +             if (gpio_is_valid(cs_gpio)) {
>> +                     if (gpio_request(cs_gpio, "ssp-pl022"))
>> +                             dev_err(&adev->dev,
>> +                                     "could not request %d gpio\n", cs_gpio);
>> +                     else if (gpio_direction_output(cs_gpio, 1))
>> +                             dev_err(&adev->dev,
>> +                                     "could set gpio %d as output\n",
>> +                                     cs_gpio);
>> +             }
>> +     }
>> +#else
>> +     for (i = 0; i < num_cs; i++)
>> +             pl022->chipselect[i] = -EINVAL;
>> +#endif
>
> ... and this would be good candidates for IS_ENABLED(CONFIG_OF).
>
>>
>>       /*
>>        * Supports mode 0-3, loopback, and active low CS. Transfers are
>> @@ -2130,6 +2204,9 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
>>   err_no_ioremap:
>>       amba_release_regions(adev);
>>   err_no_ioregion:
>> + #ifdef CONFIG_OF
>> + err_no_gpio:
>> + #endif
>
> If you use the IS_ENABLED() macro, then you no longer need these either.

Thanks for the suggestion, looks cleaner.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/