On Wed, Jun 03, 2015 at 01:49:45PM +0300, Jarkko Nikula wrote:
> static bool is_lpss_ssp(const struct driver_data *drv_data)
> {
> - return drv_data->ssp_type == LPSS_SSP;
> + return drv_data->ssp_type == LPSS_LPT_SSP ||
> + drv_data->ssp_type == LPSS_BYT_SSP;
> }
switch statement please, it helps scale new types.
> - switch (drv_data->ssp_type) {
> - case QUARK_X1000_SSP:
> + if (is_quark_x1000_ssp(drv_data)) {
> tx_thres = TX_THRESH_QUARK_X1000_DFLT;
> tx_hi_thres = 0;
> rx_thres = RX_THRESH_QUARK_X1000_DFLT;
> - break;
> - case LPSS_SSP:
> + } else if (is_lpss_ssp(drv_data)) {
Why are we moving away from a switch statement here? Half the point of
using them for variant selection is that it makes it easier to adjust
the set of cases later.
> + const struct acpi_device_id *id;
> + int devid, type = SSP_UNDEFINED;
Don't mix initialisations with other variable declarations please.
> if (!ACPI_HANDLE(&pdev->dev) ||
> acpi_bus_get_device(ACPI_HANDLE(&pdev->dev), &adev))
> return NULL;
>
> + id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
> + if (id)
> + type = (int)id->driver_data;
It'd be a bit clearer if the error case was an else here, though - on
first read I'd expected to return an error if we couldn't identify the
device and the initialisation is far enough away to appear in a
different hunk.
Attachment:
signature.asc
Description: Digital signature
