Re: [PATCH 1/3] spi: added spi_resource management

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02.12.2015 13:29, Mark Brown wrote:
On Wed, Dec 02, 2015 at 08:30:51AM +0100, Martin Sperl wrote:

This has exactly one (tiny) user.  Why is it a separate function?

Readability, mimicking devres code and the option of adding
object caching/reuse here later...

This is *not* helping readability.
Originally it contained a bit more code to avoid repeated allocations,
which was cut out for the initial version of the patch.

There is a much higher likelihood that spi_resources will be
allocated and then freed several times per second, so this
can save cpu cycles and avoid locks...

In what way does splitting this over two functions helping improve
performance?  If there were multiple users or something perhaps but
there don't appear to be.


It does not help performance per se, but it allows adding more
code in that location that would allow to reuse allocated objects.

As of now I will merge those 2 functions.

The bigger question (based on your comments to Patch 2/3) is:

Do you want to follow the devres approach (i.e: hiding
"struct spi_res" after allocation and returning "void *"
to the data-payload only in spi_res_alloc)?

Or do you prefer to have "struct spi_res" as an explicit member of
a structure (i.e. in Patch 2/3 "struct spi_res_replaced_transfers")?

I want to get this settled before submitting newer versions of the
other patches that address other concerns, because any changes to
spi_res design directly impact these other patches.

Thanks,
	Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux