netdev - Re: [linux-nfc] [PATCH 8/8] NFC: Add Intel FieldsPeak NFC solution driver

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

Message-ID: <20150524170829.GD15886@ribalta.ccr.corp.intel.com>
Date:	Sun, 24 May 2015 19:08:29 +0200
From:	Samuel Ortiz <sameo@...ux.intel.com>
To:	Robert Dolca <robert.dolca@...il.com>
Cc:	Robert Dolca <robert.dolca@...el.com>, linux-nfc@...ts.01.org,
	netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [linux-nfc] [PATCH 8/8] NFC: Add Intel FieldsPeak NFC solution
 driver

Hi Robert,

On Wed, Apr 01, 2015 at 06:35:31PM +0300, Robert Dolca wrote:
> On Thu, Mar 26, 2015 at 2:30 AM, Samuel Ortiz <sameo@...ux.intel.com> wrote:
> >> +     /* If a patch was applied the new version is checked */
> >> +     if (patched) {
> >> +             r = nci_init(ndev);
> >> +             if (r)
> >> +                     goto error;
> >> +
> >> +             r = fdp_nci_get_versions(ndev);
> >> +             if (r)
> >> +                     goto error;
> >> +
> >> +             if (info->otp_version != info->otp_patch_version ||
> >> +                 info->ram_version != info->ram_patch_version) {
> >> +                     pr_err("FRP firmware update failed");
> >> +                     r = -EINVAL;
> >> +             }
> >> +     }
> >> +
> >> +     /* Check if the device has VSC */
> >> +     if (fdp_post_fw_vsc_cfg[0]) {
> >> +             /* Set the vendor specific configuration */
> >> +             r = fdp_nci_set_production_data(ndev, fdp_post_fw_vsc_cfg[3],
> >> +                                             &fdp_post_fw_vsc_cfg[4]);
> >> +             if (r)
> >> +                     goto error;
> >> +     }
> >> +
> >> +     /* Set clock type and frequency */
> >> +     r = fdp_nci_set_clock(ndev, 0, 26000);
> >> +     if (r)
> >> +             goto error;
> > The version checking, production data setting and clock setting should
> > be part of a post setup notification call. Please add an nci_dev
> > notify() ops that could get called on certain events, for example when
> > NCI is up. Bluetooth's HCI does something along those lines already.
> > From this notification hook you could implement this post setup stage.
> >
> > The idea is for your setup routine to only do firmware update and
> > nothing else. It will make it shorter, and thus easier to read as well.
> If the RAM patch wasn't applied successfully the device can't be used
> so the setup function should fail.
> If the production data (specifically the clock frequency) is not set
> the device can not be used. If the user space tries to start polling
> before the notification is sent the polling will fail. Having it
> called later would mean introducing a race condition.
Sure. Then I'd rather have an additional NCI hook (e.g.
ndev->ops->open()) called synchronously after the setup stage that
could fail and make open fail as well. The idea here is to separate the
2 parts of your logic and make the code more readable.

Cheers,
Samuel.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html