* Re: [PATCH] fnic: remove a pointless test
From: Martin K. Petersen @ 2016-11-14 23:48 UTC (permalink / raw)
To: Tomas Henzl; +Cc: linux-scsi, sramars, hiralpat, buchino
In-Reply-To: <1478018421-22820-1-git-send-email-thenzl@redhat.com>
>>>>> "Tomas" == Tomas Henzl <thenzl@redhat.com> writes:
Tomas> rport can't be null here, it would have failed already in
Tomas> fc_remote_port_chkready
Cisco folks: Please review!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH] remote-curl: don't hang when a server dies before any output
From: Jeff King @ 2016-11-14 23:48 UTC (permalink / raw)
To: David Turner; +Cc: git@vger.kernel.org, spearce@spearce.org
In-Reply-To: <a57cc9c4a0a840baab5b8123fac9388b@exmbdft7.ad.twosigma.com>
On Mon, Nov 14, 2016 at 11:25:30PM +0000, David Turner wrote:
> > But it does seem to work. At least it doesn't seem to break anything
> > in the test suite, and it fixes the new tests you added. I'd worry
> > that there's some obscure case where the response isn't packetized
> > in the same way.
>
> Overall, this looks good to me. The state machine is pretty clean. I
> think I would have used a tiny buffer for the length field, and then I
> would have regretted it. Your way looks nicer than my unwritten patch
> would have looked.
Heh, I started it that way but you end up dealing with the same states
(they're just implicit in your "how big is my temp buffer" field).
> > +#define READ_ONE_HEX(shift) do { \
> > + int val = hexval(buf[0]); \
> > + if (val < 0) { \
> > + warning("error on %d", *buf); \
> > + rpc->pktline_state = RPC_PKTLINE_ERROR; \
> > + return; \
> > + } \
> > + rpc->pktline_len |= val << shift; \
>
> Nit: parenthesize shift here, since it is a parameter to a macro.
Yeah, I'm often a bit slack on these one-off inside-a-function macros.
But it does not hurt to to be careful.
I'll make that change and then try to wrap this up with a commit
message. I plan to steal your tests, if that's OK.
-Peff
^ permalink raw reply
* Re: [tpmdd-devel] [PATCH RFC 2/2] tpm: refactor tpm2_get_tpm_pt to tpm2_getcap_cmd
From: Jarkko Sakkinen @ 2016-11-14 23:48 UTC (permalink / raw)
To: Nayna; +Cc: moderated list:TPM DEVICE DRIVER, open list,
linux-security-module
In-Reply-To: <20161112000242.63hgv5ujmkr7hy6a@intel.com>
On Fri, Nov 11, 2016 at 04:02:43PM -0800, Jarkko Sakkinen wrote:
> On Fri, Nov 11, 2016 at 09:51:45AM +0530, Nayna wrote:
> >
> >
> > On 10/09/2016 03:44 PM, Jarkko Sakkinen wrote:
> > > Refactored tpm2_get_tpm_pt to tpm2_getcap_cmd, which means that it also
> > > takes capability ID as input. This is required to access
> > > TPM_CAP_HANDLES, which contains metadata needed for swapping transient
> > > data.
> > >
> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > ---
> > > drivers/char/tpm/tpm.h | 6 +++-
> > > drivers/char/tpm/tpm2-cmd.c | 64 ++++++++++++++++++++---------------------
> > > drivers/char/tpm/tpm_tis_core.c | 3 +-
> > > 3 files changed, 38 insertions(+), 35 deletions(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > index 0fab6d5..8176f42 100644
> > > --- a/drivers/char/tpm/tpm.h
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -85,6 +85,10 @@ enum tpm2_capabilities {
> > > TPM2_CAP_TPM_PROPERTIES = 6,
> > > };
> > >
> > > +enum tpm2_properties {
> > > + TPM2_PT_FAMILY_INDICATOR = 0x100,
> > > +};
> > > +
> > > enum tpm2_startup_types {
> > > TPM2_SU_CLEAR = 0x0000,
> > > TPM2_SU_STATE = 0x0001,
> > > @@ -485,7 +489,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
> > > int tpm2_unseal_trusted(struct tpm_chip *chip,
> > > struct trusted_key_payload *payload,
> > > struct trusted_key_options *options);
> > > -ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
> > > +ssize_t tpm2_getcap_cmd(struct tpm_chip *chip, u32 cap_id, u32 property_id,
> > > u32 *value, const char *desc);
> > >
> > > int tpm2_auto_startup(struct tpm_chip *chip);
> > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> > > index 2900e18..fcf3d86 100644
> > > --- a/drivers/char/tpm/tpm2-cmd.c
> > > +++ b/drivers/char/tpm/tpm2-cmd.c
> > > @@ -111,13 +111,13 @@ struct tpm2_pcr_extend_in {
> > > u8 digest[TPM_DIGEST_SIZE];
> > > } __packed;
> > >
> > > -struct tpm2_get_tpm_pt_in {
> > > +struct tpm2_getcap_in {
> > > __be32 cap_id;
> > > __be32 property_id;
> > > __be32 property_cnt;
> > > } __packed;
> > >
> > > -struct tpm2_get_tpm_pt_out {
> > > +struct tpm2_getcap_out {
> > > u8 more_data;
> > > __be32 subcap_id;
> > > __be32 property_cnt;
> > > @@ -140,8 +140,8 @@ union tpm2_cmd_params {
> > > struct tpm2_pcr_read_in pcrread_in;
> > > struct tpm2_pcr_read_out pcrread_out;
> > > struct tpm2_pcr_extend_in pcrextend_in;
> > > - struct tpm2_get_tpm_pt_in get_tpm_pt_in;
> > > - struct tpm2_get_tpm_pt_out get_tpm_pt_out;
> > > + struct tpm2_getcap_in getcap_in;
> > > + struct tpm2_getcap_out getcap_out;
> > > struct tpm2_get_random_in getrandom_in;
> > > struct tpm2_get_random_out getrandom_out;
> > > };
> > > @@ -435,16 +435,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> > > return total ? total : -EIO;
> > > }
> > >
> > > -#define TPM2_GET_TPM_PT_IN_SIZE \
> > > - (sizeof(struct tpm_input_header) + \
> > > - sizeof(struct tpm2_get_tpm_pt_in))
> > > -
> > > -static const struct tpm_input_header tpm2_get_tpm_pt_header = {
> > > - .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> > > - .length = cpu_to_be32(TPM2_GET_TPM_PT_IN_SIZE),
> > > - .ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
> > > -};
> > > -
> > > /**
> > > * Append TPMS_AUTH_COMMAND to the buffer. The buffer must be allocated with
> > > * tpm_buf_alloc().
> > > @@ -750,35 +740,43 @@ out:
> > > return rc;
> > > }
> > >
> > > +#define TPM2_GETCAP_IN_SIZE \
> > > + (sizeof(struct tpm_input_header) + sizeof(struct tpm2_getcap_in))
> > > +
> > > +static const struct tpm_input_header tpm2_getcap_header = {
> > > + .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> > > + .length = cpu_to_be32(TPM2_GETCAP_IN_SIZE),
> > > + .ordinal = cpu_to_be32(TPM2_CC_GET_CAPABILITY)
> > > +};
> > > +
> > > /**
> > > - * tpm2_get_tpm_pt() - get value of a TPM_CAP_TPM_PROPERTIES type property
> > > - * @chip: TPM chip to use.
> > > - * @property_id: property ID.
> > > - * @value: output variable.
> > > + * tpm2_getcap_cmd() - execute a TPM2_GetCapability command
> > > + * @chip: TPM chip to use
> > > + * @cap_id: capability ID
> > > + * @property_id: property ID
> > > + * @value: value of the property
> > > * @desc: passed to tpm_transmit_cmd()
> > > *
> > > - * 0 is returned when the operation is successful. If a negative number is
> > > - * returned it remarks a POSIX error code. If a positive number is returned
> > > - * it remarks a TPM error.
> > > + * Return: same as with tpm_transmit_cmd
> > > */
> > > -ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value,
> > > - const char *desc)
> > > +ssize_t tpm2_getcap_cmd(struct tpm_chip *chip, u32 cap_id, u32 property_id,
> > > + u32 *value, const char *desc)
> >
> > This function currently returns single value "u32 *value" as output data.
> >
> > Some calling function expect list of values from capabilities output.
> > For eg., get_active_banks() use TPM_CAP_PCRS capability to retrieve list of
> > active banks. And this capability returns array of pcr selections(which is a
> > struct representing each active PCR bank)
> >
> > I was thinking, can we define output parameter as struct of cap_id and union
> > of expected cap_data ?
>
> Unless you have major concerns about performance, which I think are not
> relevant here, calling this in a loop is perfectly adequate and a lot of
> simpler than having a generic version (with moreData handling and
> everything).
>
> I would rather see uses of struct cap_t to shrink than to expand. The
> same goes for other horrific unions that exist today in the driver.
If you are fine with this patch (give Reviewed-by) I can apply this
and add to my 4.10 pull request so that we have common baseline to
develop both event log and the resource manager.
This is a very low risk commit to take to 4.10 because it is only used
for interrupt generation in the TIS driver at the moment.
/Jarkko
^ permalink raw reply
* Re: [PATCH] netfilter: Update ip_route_me_harder to consider L3 domain
From: Pablo Neira Ayuso @ 2016-11-14 23:48 UTC (permalink / raw)
To: David Ahern; +Cc: kaber, kadlec, netfilter-devel
In-Reply-To: <6698ac14-9706-88b3-ed1f-0ec47c64fb48@cumulusnetworks.com>
Hi David,
On Mon, Nov 14, 2016 at 04:04:26PM -0700, David Ahern wrote:
> On 11/14/16 3:59 PM, Pablo Neira Ayuso wrote:
> > Does ip6_route_me_harder need an update too?
>
> I have not hit a use case yet. Rather than blindly going through and
> adding l3mdev hooks I would like to tie the changes to known uses
> cases.
Hm, your follow up patch updates nf_send_reset6() but not
nf_send_reset(). Sorry but it strikes me as inconsistent that some
spots are updated and some others are not.
^ permalink raw reply
* Re: [PATCH] megaraid-sas: request irqs later
From: Martin K. Petersen @ 2016-11-14 23:48 UTC (permalink / raw)
To: Tomas Henzl; +Cc: linux-scsi, sumit.saxena, kashyap.desai
In-Reply-To: <1478017922-22655-1-git-send-email-thenzl@redhat.com>
>>>>> "Tomas" == Tomas Henzl <thenzl@redhat.com> writes:
Tomas> It is not good when an irq arrives before driver structures are
Tomas> allocated.
Sumit, Kashyap: Please review!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: qed, qedi patchset submission
From: Martin K. Petersen @ 2016-11-14 23:47 UTC (permalink / raw)
To: Arun Easi; +Cc: Martin K. Petersen, David Miller, linux-scsi, netdev
In-Reply-To: <alpine.LRH.2.00.1611141256340.28058@mvluser05.qlc.com>
>>>>> "Arun" == Arun Easi <arun.easi@cavium.com> writes:
Arun,
Arun> Do you have any preference or thoughts on how the "qed" patches be
Arun> approached? Just as a reference, our rdma driver "qedr" went
Arun> through something similar[1], and eventually "qed" patches were
Arun> taken by David in the net tree and "qedr", in the rdma tree
Arun> (obviously) by Doug L.
DaveM can queue the whole series or I can hold the SCSI pieces for
rc1. Either way works for me.
However, I would like to do a review of the driver first. I will try to
get it done this week.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: qed, qedi patchset submission
From: Martin K. Petersen @ 2016-11-14 23:47 UTC (permalink / raw)
To: Arun Easi; +Cc: Martin K. Petersen, David Miller, linux-scsi, netdev
In-Reply-To: <alpine.LRH.2.00.1611141256340.28058@mvluser05.qlc.com>
>>>>> "Arun" == Arun Easi <arun.easi@cavium.com> writes:
Arun,
Arun> Do you have any preference or thoughts on how the "qed" patches be
Arun> approached? Just as a reference, our rdma driver "qedr" went
Arun> through something similar[1], and eventually "qed" patches were
Arun> taken by David in the net tree and "qedr", in the rdma tree
Arun> (obviously) by Doug L.
DaveM can queue the whole series or I can hold the SCSI pieces for
rc1. Either way works for me.
However, I would like to do a review of the driver first. I will try to
get it done this week.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 03/21] hbitmap: improve dirty iter
From: John Snow @ 2016-11-14 23:47 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block, qemu-devel
Cc: kwolf, famz, armbru, mreitz, stefanha, pbonzini, den
In-Reply-To: <1478715476-132280-4-git-send-email-vsementsov@virtuozzo.com>
On 11/09/2016 01:17 PM, Vladimir Sementsov-Ogievskiy wrote:
> Make dirty iter resistant to resetting bits in corresponding HBitmap.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> include/qemu/hbitmap.h | 25 +++----------------------
> util/hbitmap.c | 23 ++++++++++++++++++++++-
> 2 files changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
> index eb46475..594f6f8 100644
> --- a/include/qemu/hbitmap.h
> +++ b/include/qemu/hbitmap.h
> @@ -243,10 +243,8 @@ void hbitmap_free(HBitmap *hb);
> * the lowest-numbered bit that is set in @hb, starting at @first.
> *
> * Concurrent setting of bits is acceptable, and will at worst cause the
> - * iteration to miss some of those bits. Resetting bits before the current
> - * position of the iterator is also okay. However, concurrent resetting of
> - * bits can lead to unexpected behavior if the iterator has not yet reached
> - * those bits.
> + * iteration to miss some of those bits. Concurrent resetting of bits is OK,
> + * too.
> */
Minor bikeshed: I might remove "too" because the concurrent resetting of
bits is perfectly ok in /contrast/ to the concurrent setting of bits,
which has undesirable side-effects.
I may simply strongly word this as:
"The concurrent resetting of bits is OK."
It's not worth a rewrite on its own.
Reviewed-by: John Snow <jsnow@redhat.com>
> void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first);
>
> @@ -285,24 +283,7 @@ void hbitmap_free_meta(HBitmap *hb);
> * Return the next bit that is set in @hbi's associated HBitmap,
> * or -1 if all remaining bits are zero.
> */
> -static inline int64_t hbitmap_iter_next(HBitmapIter *hbi)
> -{
> - unsigned long cur = hbi->cur[HBITMAP_LEVELS - 1];
> - int64_t item;
> -
> - if (cur == 0) {
> - cur = hbitmap_iter_skip_words(hbi);
> - if (cur == 0) {
> - return -1;
> - }
> - }
> -
> - /* The next call will resume work from the next bit. */
> - hbi->cur[HBITMAP_LEVELS - 1] = cur & (cur - 1);
> - item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ctzl(cur);
> -
> - return item << hbi->granularity;
> -}
> +int64_t hbitmap_iter_next(HBitmapIter *hbi);
>
> /**
> * hbitmap_iter_next_word:
> diff --git a/util/hbitmap.c b/util/hbitmap.c
> index 5d1a21c..48d8b2d 100644
> --- a/util/hbitmap.c
> +++ b/util/hbitmap.c
> @@ -106,8 +106,9 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi)
>
> unsigned long cur;
> do {
> - cur = hbi->cur[--i];
> + i--;
> pos >>= BITS_PER_LEVEL;
> + cur = hbi->cur[i] & hb->levels[i][pos];
> } while (cur == 0);
>
> /* Check for end of iteration. We always use fewer than BITS_PER_LONG
> @@ -139,6 +140,26 @@ unsigned long hbitmap_iter_skip_words(HBitmapIter *hbi)
> return cur;
> }
>
> +int64_t hbitmap_iter_next(HBitmapIter *hbi)
> +{
> + unsigned long cur = hbi->cur[HBITMAP_LEVELS - 1] &
> + hbi->hb->levels[HBITMAP_LEVELS - 1][hbi->pos];
> + int64_t item;
> +
> + if (cur == 0) {
> + cur = hbitmap_iter_skip_words(hbi);
> + if (cur == 0) {
> + return -1;
> + }
> + }
> +
> + /* The next call will resume work from the next bit. */
> + hbi->cur[HBITMAP_LEVELS - 1] = cur & (cur - 1);
> + item = ((uint64_t)hbi->pos << BITS_PER_LEVEL) + ctzl(cur);
> +
> + return item << hbi->granularity;
> +}
> +
> void hbitmap_iter_init(HBitmapIter *hbi, const HBitmap *hb, uint64_t first)
> {
> unsigned i, bit;
>
^ permalink raw reply
* Re: frame-pointer based user stack unwinding with perf on arm32
From: Kim Phillips @ 2016-11-14 23:45 UTC (permalink / raw)
To: Milian Wolff; +Cc: perf group
In-Reply-To: <2484144.z8iE8SUiXE@milian-kdab2>
On Mon, 14 Nov 2016 14:14:30 +0100
Milian Wolff <milian.wolff@kdab.com> wrote:
Hi Milian,
> None of these produced the desired results when running `perf record -g` on
> the target platform (a panda board):
>
> root@arm:~# perf record -g ./stress_bt
> Total count: 171711327751528502
> root@arm:~# perf script
> <snip>
> ...
> stress_bt 825 7645.3346298627: 8241360 cycles:ppp:
> 5a0 foo_128+0xfffe0084 (/root/stress_bt)
>
> stress_bt 825 7645.3346305738: 7932022 cycles:ppp:
> 592 foo_128+0xfffe0076 (/root/stress_bt)
> ...
> So, can someone please clarify whether this should also work on arm32? What
> are the requirements?
I have this working with a natively-built perf (today's acme's
perf/core branch):
$ ./perf --version
perf version 4.9.rc1.g699c
$ uname -a
Linux tc2 4.8.0+ #7 SMP Tue Oct 4 10:29:55 CDT 2016 armv7l GNU/Linux
$ cat ./runcallg.sh
sudo ./perf record -o perf.data --call-graph dwarf -- ./stress_bt |& tee record-callg.log
sudo ./perf report --call-graph --stdio >& report-callg.log
$ ./runcallg.sh
Lowering default frequency rate to 1600.
Please consider tweaking /proc/sys/kernel/perf_event_max_sample_rate.
Total count: 171711327751528502
[ perf record: Woken up 514 times to write data ]
Warning:
Processed 19294 events and lost 32 chunks!
Check IO/CPU overload!
[ perf record: Captured and wrote 128.420 MB perf.data (16025 samples) ]
$ head -40 report-callg.log
Warning:
Processed 19294 events and lost 32 chunks!
Check IO/CPU overload!
# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 16K of event 'cycles:ppp'
# Event count (approx.): 7863498267
#
# Children Self Command Shared Object Symbol
# ........ ........ ......... ................. ..................................
#
99.62% 99.58% stress_bt stress_bt [.] foo_128
|
|--95.74%--__libc_start_main
| main
| doit
| bar
| |
| |--1.01%--foo_30
| | foo_31
| | foo_32
| | foo_33
| | foo_34
| | foo_35
| | foo_36
| | foo_37
| | foo_38
| | foo_39
| | foo_40
| | foo_41
| | foo_42
| | foo_43
| | foo_44
| | foo_45
| | foo_46
The above works both with the arm32 binary included in the downloaded
stress_bt.tar.gz, and one built with a native gcc 4.9.2, using only the
'-g' flag ("gcc -g stress_bt.c").
OTOH, I tried using a cross-built perf, and it did not work (same
behaviour you're seeing).
The Linaro wiki page lists at least libunwind as a dependency, and the
native build has it:
Auto-detecting system features:
... dwarf: [ on ]
... dwarf_getlocations: [ on ]
... glibc: [ on ]
... gtk2: [ OFF ]
... libaudit: [ on ]
... libbfd: [ on ]
... libelf: [ on ]
... libnuma: [ OFF ]
... numa_num_possible_cpus: [ OFF ]
... libperl: [ on ]
... libpython: [ on ]
... libslang: [ on ]
... libcrypto: [ on ]
... libunwind: [ on ]
... libdw-dwarf-unwind: [ on ]
... zlib: [ on ]
... lzma: [ on ]
... get_cpuid: [ OFF ]
... bpf: [ OFF ]
Makefile.config:349: BPF prologue is not supported by architecture arm, missing regs_query_register_offset()
Makefile.config:422: BPF API too old. Please install recent kernel headers. BPF support in 'perf record' is disabled.
Makefile.config:519: GTK2 not found, disables GTK2 support. Please install gtk2-devel or libgtk2.0-dev
Makefile.config:693: No numa.h found, disables 'perf bench numa mem' benchmark, please install numactl-devel/libnuma-devel/libnuma-dev
whereas the cross build does not:
Auto-detecting system features:
... dwarf: [ OFF ]
... dwarf_getlocations: [ OFF ]
... glibc: [ on ]
... gtk2: [ OFF ]
... libaudit: [ OFF ]
... libbfd: [ OFF ]
... libelf: [ OFF ]
... libnuma: [ OFF ]
... numa_num_possible_cpus: [ OFF ]
... libperl: [ OFF ]
... libpython: [ OFF ]
... libslang: [ OFF ]
... libcrypto: [ OFF ]
... libunwind: [ OFF ]
... libdw-dwarf-unwind: [ OFF ]
... zlib: [ OFF ]
... lzma: [ OFF ]
... get_cpuid: [ OFF ]
... bpf: [ on ]
Makefile.config:260: No libelf found, disables 'probe' tool and BPF support in 'perf record', please install libelf-dev, l
ibelf-devel or elfutils-libelf-devel
Makefile.config:360: No sys/sdt.h found, no SDT events are defined, please install systemtap-sdt-devel or systemtap-sdt-de
v
Makefile.config:433: Disabling post unwind, no support found.
Makefile.config:479: No libaudit.h found, disables 'trace' tool, please install audit-libs-devel or libaudit-dev
Makefile.config:490: No libcrypto.h found, disables jitted code injection, please install libssl-devel or libssl-dev
Makefile.config:505: slang not found, disables TUI support. Please install slang-devel, libslang-dev or libslang2-dev
Makefile.config:519: GTK2 not found, disables GTK2 support. Please install gtk2-devel or libgtk2.0-dev
Makefile.config:547: Missing perl devel files. Disabling perl scripting support, please install perl-ExtUtils-Embed/libper
l-dev
Makefile.config:590: No 'Python.h' (for Python 2.x support) was found: disables Python support - please install python-dev
el/python-dev
Makefile.config:680: No liblzma found, disables xz kernel module decompression, please install xz-devel/liblzma-dev
Makefile.config:693: No numa.h found, disables 'perf bench numa mem' benchmark, please install numactl-devel/libnuma-devel
/libnuma-dev
Unfortunately, I don't know how to cross-build perf with libunwind
turned on: On Ubuntu, I cd tools/ and issue 'make ARCH=arm
CROSS_COMPILE=arm-linux-gnueabihf- perf'. Installing something called
android-libunwind-dev didn't help, and I can't tell whether the wiki
page includes building perf in a cross environment (in fact, it
references a /lib/arm-linux-gnueabihf/ which is present on my versatile
express' target Debian installation).
hth,
Kim
^ permalink raw reply
* [PATCH] tpm: drop chip->is_open and chip->duration_adjusted
From: Jarkko Sakkinen @ 2016-11-14 23:44 UTC (permalink / raw)
To: tpmdd-devel
Cc: linux-security-module, Jarkko Sakkinen, Peter Huewe,
Marcel Selhorst, Jason Gunthorpe, Christophe Ricard, open list
Use atomic bitops for chip->flags so that we do not need chip->is_open
and chip->duration_adjusted anymore.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
drivers/char/tpm/st33zp24/st33zp24.c | 6 +++---
drivers/char/tpm/tpm-chip.c | 14 ++++++++------
drivers/char/tpm/tpm-dev.c | 9 +++------
drivers/char/tpm/tpm-interface.c | 30 +++++++++++++++---------------
drivers/char/tpm/tpm-sysfs.c | 2 +-
drivers/char/tpm/tpm.h | 14 +++++++-------
drivers/char/tpm/tpm2-cmd.c | 2 +-
drivers/char/tpm/tpm_crb.c | 2 +-
drivers/char/tpm/tpm_i2c_nuvoton.c | 8 ++++----
drivers/char/tpm/tpm_tis_core.c | 26 +++++++++++++-------------
drivers/char/tpm/tpm_vtpm_proxy.c | 2 +-
11 files changed, 57 insertions(+), 58 deletions(-)
diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
index 6f060c7..14734a0 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -267,7 +267,7 @@ static int wait_for_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
stop = jiffies + timeout;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) {
cur_intrs = tpm_dev->intrs;
clear_interruption(tpm_dev);
enable_irq(tpm_dev->irq);
@@ -429,7 +429,7 @@ static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
if (ret < 0)
goto out_err;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) {
ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
ret = wait_for_stat(chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
@@ -586,7 +586,7 @@ int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops,
goto _tpm_clean_answer;
tpm_dev->irq = irq;
- chip->flags |= TPM_CHIP_FLAG_IRQ;
+ set_bit(TPM_CHIP_FLAG_IRQ, &chip->flags);
disable_irq_nosync(tpm_dev->irq);
}
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 3f27753..04819e1 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -183,7 +183,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
goto out;
if (!dev)
- chip->flags |= TPM_CHIP_FLAG_VIRTUAL;
+ set_bit(TPM_CHIP_FLAG_VIRTUAL, &chip->flags);
cdev_init(&chip->cdev, &tpm_fops);
chip->cdev.owner = THIS_MODULE;
@@ -271,7 +271,7 @@ static void tpm_del_char_device(struct tpm_chip *chip)
/* Make the driver uncallable. */
down_write(&chip->ops_sem);
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
tpm2_shutdown(chip, TPM2_SU_CLEAR);
chip->ops = NULL;
up_write(&chip->ops_sem);
@@ -281,7 +281,8 @@ static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
{
struct attribute **i;
- if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags) ||
+ test_bit(TPM_CHIP_FLAG_VIRTUAL, &chip->flags))
return;
sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
@@ -299,8 +300,9 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
struct attribute **i;
int rc;
- if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
- return 0;
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags) ||
+ test_bit(TPM_CHIP_FLAG_VIRTUAL, &chip->flags))
+ return;
rc = __compat_only_sysfs_link_entry_to_kobj(
&chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
@@ -335,7 +337,7 @@ int tpm_chip_register(struct tpm_chip *chip)
int rc;
if (chip->ops->flags & TPM_OPS_AUTO_STARTUP) {
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
rc = tpm2_auto_startup(chip);
else
rc = tpm1_auto_startup(chip);
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 912ad30..3738c38 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -57,17 +57,14 @@ static int tpm_open(struct inode *inode, struct file *file)
container_of(inode->i_cdev, struct tpm_chip, cdev);
struct file_priv *priv;
- /* It's assured that the chip will be opened just once,
- * by the check of is_open variable, which is protected
- * by driver_lock. */
- if (test_and_set_bit(0, &chip->is_open)) {
+ if (test_and_set_bit(TPM_CHIP_FLAG_IS_OPEN, &chip->flags)) {
dev_dbg(&chip->dev, "Another process owns this TPM\n");
return -EBUSY;
}
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (priv == NULL) {
- clear_bit(0, &chip->is_open);
+ clear_bit(TPM_CHIP_FLAG_IS_OPEN, &chip->flags);
return -ENOMEM;
}
@@ -173,7 +170,7 @@ static int tpm_release(struct inode *inode, struct file *file)
flush_work(&priv->work);
file->private_data = NULL;
atomic_set(&priv->data_pending, 0);
- clear_bit(0, &priv->chip->is_open);
+ clear_bit(TPM_CHIP_FLAG_IS_OPEN, &priv->chip->flags);
kfree(priv);
return 0;
}
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a2688ac..6426be7 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -367,10 +367,10 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
goto out;
}
- if (chip->flags & TPM_CHIP_FLAG_IRQ)
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags))
goto out_recv;
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
stop = jiffies + tpm2_calc_ordinal_duration(chip, ordinal);
else
stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
@@ -503,10 +503,10 @@ int tpm_get_timeouts(struct tpm_chip *chip)
unsigned long old_timeout[4];
ssize_t rc;
- if (chip->flags & TPM_CHIP_FLAG_HAVE_TIMEOUTS)
+ if (test_bit(TPM_CHIP_FLAG_HAVE_TIMEOUTS, &chip->flags))
return 0;
- if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) {
/* Fixed timeouts for TPM2 */
chip->timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A);
chip->timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B);
@@ -519,7 +519,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
chip->duration[TPM_LONG] =
msecs_to_jiffies(TPM2_DURATION_LONG);
- chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
+ set_bit(TPM_CHIP_FLAG_HAVE_TIMEOUTS, &chip->flags);
return 0;
}
@@ -600,11 +600,11 @@ int tpm_get_timeouts(struct tpm_chip *chip)
chip->duration[TPM_SHORT] = HZ;
chip->duration[TPM_MEDIUM] *= 1000;
chip->duration[TPM_LONG] *= 1000;
- chip->duration_adjusted = true;
+ set_bit(TPM_CHIP_FLAG_DURATION_ADJUSTED, &chip->flags);
dev_info(&chip->dev, "Adjusting TPM timeout parameters.");
}
- chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
+ set_bit(TPM_CHIP_FLAG_HAVE_TIMEOUTS, &chip->flags);
return 0;
}
EXPORT_SYMBOL_GPL(tpm_get_timeouts);
@@ -676,7 +676,7 @@ int tpm_is_tpm2(u32 chip_num)
if (chip == NULL)
return -ENODEV;
- rc = (chip->flags & TPM_CHIP_FLAG_TPM2) != 0;
+ rc = (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) != 0;
tpm_put_ops(chip);
@@ -703,7 +703,7 @@ int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
chip = tpm_chip_find_get(chip_num);
if (chip == NULL)
return -ENODEV;
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
else
rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
@@ -740,7 +740,7 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
if (chip == NULL)
return -ENODEV;
- if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) {
rc = tpm2_pcr_extend(chip, pcr_idx, hash);
tpm_put_ops(chip);
return rc;
@@ -890,7 +890,7 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
stop = jiffies + timeout;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) {
again:
timeout = stop - jiffies;
if ((long)timeout <= 0)
@@ -944,7 +944,7 @@ int tpm_pm_suspend(struct device *dev)
if (chip == NULL)
return -ENODEV;
- if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) {
tpm2_shutdown(chip, TPM2_SU_STATE);
return 0;
}
@@ -1036,7 +1036,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
if (chip == NULL)
return -ENODEV;
- if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) {
err = tpm2_get_random(chip, out, max);
tpm_put_ops(chip);
return err;
@@ -1081,7 +1081,7 @@ int tpm_seal_trusted(u32 chip_num, struct trusted_key_payload *payload,
int rc;
chip = tpm_chip_find_get(chip_num);
- if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
+ if (chip == NULL || !(test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)))
return -ENODEV;
rc = tpm2_seal_trusted(chip, payload, options);
@@ -1107,7 +1107,7 @@ int tpm_unseal_trusted(u32 chip_num, struct trusted_key_payload *payload,
int rc;
chip = tpm_chip_find_get(chip_num);
- if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
+ if (chip == NULL || !(test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)))
return -ENODEV;
rc = tpm2_unseal_trusted(chip, payload, options);
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 848ad65..21b3fde 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -244,7 +244,7 @@ static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
jiffies_to_usecs(chip->duration[TPM_SHORT]),
jiffies_to_usecs(chip->duration[TPM_MEDIUM]),
jiffies_to_usecs(chip->duration[TPM_LONG]),
- chip->duration_adjusted
+ test_bit(TPM_CHIP_FLAG_DURATION_ADJUSTED, &chip->flags)
? "adjusted" : "original");
}
static DEVICE_ATTR_RO(durations);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1ae9768..a1fadfa 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -142,10 +142,12 @@ enum tpm2_startup_types {
#define TPM_PPI_VERSION_LEN 3
enum tpm_chip_flags {
- TPM_CHIP_FLAG_TPM2 = BIT(1),
- TPM_CHIP_FLAG_IRQ = BIT(2),
- TPM_CHIP_FLAG_VIRTUAL = BIT(3),
- TPM_CHIP_FLAG_HAVE_TIMEOUTS = BIT(4),
+ TPM_CHIP_FLAG_IS_OPEN = 0,
+ TPM_CHIP_FLAG_TPM2 = 1,
+ TPM_CHIP_FLAG_IRQ = 2,
+ TPM_CHIP_FLAG_VIRTUAL = 3,
+ TPM_CHIP_FLAG_HAVE_TIMEOUTS = 4,
+ TPM_CHIP_FLAG_DURATION_ADJUSTED = 5,
};
struct tpm_chip_seqops {
@@ -168,10 +170,9 @@ struct tpm_chip {
struct tpm_chip_seqops bin_log_seqops;
struct tpm_chip_seqops ascii_log_seqops;
- unsigned int flags;
+ unsigned long flags;
int dev_num; /* /dev/tpm# */
- unsigned long is_open; /* only one allowed */
struct mutex tpm_mutex; /* tpm is processing */
@@ -181,7 +182,6 @@ struct tpm_chip {
unsigned long timeout_d; /* jiffies */
bool timeout_adjusted;
unsigned long duration[3]; /* jiffies */
- bool duration_adjusted;
struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index da5b782..7abe8a0 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -946,7 +946,7 @@ int tpm2_probe(struct tpm_chip *chip)
return rc;
if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
- chip->flags |= TPM_CHIP_FLAG_TPM2;
+ set_bit(TPM_CHIP_FLAG_TPM2, &chip->flags);
return 0;
}
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 65040d7..e37ebaa 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -429,7 +429,7 @@ static int crb_acpi_add(struct acpi_device *device)
dev_set_drvdata(&chip->dev, priv);
chip->acpi_dev_handle = device->handle;
- chip->flags = TPM_CHIP_FLAG_TPM2;
+ set_bit(TPM_CHIP_FLAG_TPM2, &chip->flags);
rc = crb_cmd_ready(dev, priv);
if (rc)
diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
index e3a9155..f4e25b5 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -180,7 +180,7 @@ static bool i2c_nuvoton_check_status(struct tpm_chip *chip, u8 mask, u8 value)
static int i2c_nuvoton_wait_for_stat(struct tpm_chip *chip, u8 mask, u8 value,
u32 timeout, wait_queue_head_t *queue)
{
- if ((chip->flags & TPM_CHIP_FLAG_IRQ) && queue) {
+ if ((test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) && queue) {
s32 rc;
struct priv_data *priv = dev_get_drvdata(&chip->dev);
unsigned int cur_intrs = priv->intrs;
@@ -552,10 +552,10 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
of_id = of_match_device(dev->driver->of_match_table, dev);
if (of_id && of_id->data == OF_IS_TPM2)
- chip->flags |= TPM_CHIP_FLAG_TPM2;
+ set_bit(TPM_CHIP_FLAG_TPM2, &chip->flags);
} else
if (id->driver_data == I2C_IS_TPM2)
- chip->flags |= TPM_CHIP_FLAG_TPM2;
+ set_bit(TPM_CHIP_FLAG_TPM2, &chip->flags);
init_waitqueue_head(&priv->read_queue);
@@ -585,7 +585,7 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
__func__, priv->irq);
priv->irq = 0;
} else {
- chip->flags |= TPM_CHIP_FLAG_IRQ;
+ set_bit(TPM_CHIP_FLAG_IRQ, &chip->flags);
/* Clear any pending interrupt */
i2c_nuvoton_ready(chip);
/* - wait for TPM_STS==0xA0 (stsValid, commandReady) */
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 7993678..e6af2a0 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -105,7 +105,7 @@ static int request_locality(struct tpm_chip *chip, int l)
stop = jiffies + chip->timeout_a;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) {
again:
timeout = stop - jiffies;
if ((long)timeout <= 0)
@@ -346,7 +346,7 @@ static void disable_interrupts(struct tpm_chip *chip)
devm_free_irq(chip->dev.parent, priv->irq, chip);
priv->irq = 0;
- chip->flags &= ~TPM_CHIP_FLAG_IRQ;
+ clear_bit(TPM_CHIP_FLAG_IRQ, &chip->flags);
}
/*
@@ -370,10 +370,10 @@ static int tpm_tis_send_main(struct tpm_chip *chip, u8 *buf, size_t len)
if (rc < 0)
goto out_err;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) {
ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
dur = tpm2_calc_ordinal_duration(chip, ordinal);
else
dur = tpm_calc_ordinal_duration(chip, ordinal);
@@ -397,16 +397,16 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
int rc, irq;
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
- if (!(chip->flags & TPM_CHIP_FLAG_IRQ) || priv->irq_tested)
+ if (!(test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) || priv->irq_tested)
return tpm_tis_send_main(chip, buf, len);
/* Verify receipt of the expected IRQ */
irq = priv->irq;
priv->irq = 0;
- chip->flags &= ~TPM_CHIP_FLAG_IRQ;
+ clear_bit(TPM_CHIP_FLAG_IRQ, &chip->flags);
rc = tpm_tis_send_main(chip, buf, len);
priv->irq = irq;
- chip->flags |= TPM_CHIP_FLAG_IRQ;
+ set_bit(TPM_CHIP_FLAG_IRQ, &chip->flags);
if (!priv->irq_tested)
msleep(1);
if (!priv->irq_tested)
@@ -549,7 +549,7 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
u32 cap2;
cap_t cap;
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
return tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
else
return tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc);
@@ -611,7 +611,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
/* tpm_tis_send will either confirm the interrupt is working or it
* will call disable_irq which undoes all of the above.
*/
- if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
+ if (!(test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags))) {
rc = tpm_tis_write8(priv, original_int_vec,
TPM_INT_VECTOR(priv->locality));
if (rc < 0)
@@ -737,7 +737,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
goto out_err;
dev_info(dev, "%s TPM (device-id 0x%X, rev-id %d)\n",
- (chip->flags & TPM_CHIP_FLAG_TPM2) ? "2.0" : "1.2",
+ (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) ? "2.0" : "1.2",
vendor >> 16, rid);
if (!(priv->flags & TPM_TIS_ITPM_POSSIBLE)) {
@@ -794,7 +794,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
if (irq) {
tpm_tis_probe_irq_single(chip, intmask, IRQF_SHARED,
irq);
- if (!(chip->flags & TPM_CHIP_FLAG_IRQ))
+ if (!(test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)))
dev_err(&chip->dev, FW_BUG
"TPM interrupt not working, polling instead\n");
} else {
@@ -839,7 +839,7 @@ int tpm_tis_resume(struct device *dev)
struct tpm_chip *chip = dev_get_drvdata(dev);
int ret;
- if (chip->flags & TPM_CHIP_FLAG_IRQ)
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags))
tpm_tis_reenable_interrupts(chip);
ret = tpm_pm_resume(dev);
@@ -849,7 +849,7 @@ int tpm_tis_resume(struct device *dev)
/* TPM 1.2 requires self-test on resume. This function actually returns
* an error code but for unknown reason it isn't handled.
*/
- if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+ if (!(test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)))
tpm_do_selftest(chip);
return 0;
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 3d6f6ca..b0cb284 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -482,7 +482,7 @@ static struct file *vtpm_proxy_create_device(
vtpm_proxy_fops_open(file);
if (proxy_dev->flags & VTPM_PROXY_FLAG_TPM2)
- proxy_dev->chip->flags |= TPM_CHIP_FLAG_TPM2;
+ set_bit(TPM_CHIP_FLAG_TPM2, &proxy_dev->chip->flags);
vtpm_proxy_work_start(proxy_dev);
--
2.9.3
^ permalink raw reply related
* [PATCH] tpm: drop chip->is_open and chip->duration_adjusted
From: Jarkko Sakkinen @ 2016-11-14 23:44 UTC (permalink / raw)
To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
Cc: Christophe Ricard, open list,
linux-security-module-u79uwXL29TY76Z2rM5mHXA
Use atomic bitops for chip->flags so that we do not need chip->is_open
and chip->duration_adjusted anymore.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/char/tpm/st33zp24/st33zp24.c | 6 +++---
drivers/char/tpm/tpm-chip.c | 14 ++++++++------
drivers/char/tpm/tpm-dev.c | 9 +++------
drivers/char/tpm/tpm-interface.c | 30 +++++++++++++++---------------
drivers/char/tpm/tpm-sysfs.c | 2 +-
drivers/char/tpm/tpm.h | 14 +++++++-------
drivers/char/tpm/tpm2-cmd.c | 2 +-
drivers/char/tpm/tpm_crb.c | 2 +-
drivers/char/tpm/tpm_i2c_nuvoton.c | 8 ++++----
drivers/char/tpm/tpm_tis_core.c | 26 +++++++++++++-------------
drivers/char/tpm/tpm_vtpm_proxy.c | 2 +-
11 files changed, 57 insertions(+), 58 deletions(-)
diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c
index 6f060c7..14734a0 100644
--- a/drivers/char/tpm/st33zp24/st33zp24.c
+++ b/drivers/char/tpm/st33zp24/st33zp24.c
@@ -267,7 +267,7 @@ static int wait_for_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
stop = jiffies + timeout;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) {
cur_intrs = tpm_dev->intrs;
clear_interruption(tpm_dev);
enable_irq(tpm_dev->irq);
@@ -429,7 +429,7 @@ static int st33zp24_send(struct tpm_chip *chip, unsigned char *buf,
if (ret < 0)
goto out_err;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) {
ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
ret = wait_for_stat(chip, TPM_STS_DATA_AVAIL | TPM_STS_VALID,
@@ -586,7 +586,7 @@ int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops,
goto _tpm_clean_answer;
tpm_dev->irq = irq;
- chip->flags |= TPM_CHIP_FLAG_IRQ;
+ set_bit(TPM_CHIP_FLAG_IRQ, &chip->flags);
disable_irq_nosync(tpm_dev->irq);
}
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 3f27753..04819e1 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -183,7 +183,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
goto out;
if (!dev)
- chip->flags |= TPM_CHIP_FLAG_VIRTUAL;
+ set_bit(TPM_CHIP_FLAG_VIRTUAL, &chip->flags);
cdev_init(&chip->cdev, &tpm_fops);
chip->cdev.owner = THIS_MODULE;
@@ -271,7 +271,7 @@ static void tpm_del_char_device(struct tpm_chip *chip)
/* Make the driver uncallable. */
down_write(&chip->ops_sem);
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
tpm2_shutdown(chip, TPM2_SU_CLEAR);
chip->ops = NULL;
up_write(&chip->ops_sem);
@@ -281,7 +281,8 @@ static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
{
struct attribute **i;
- if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags) ||
+ test_bit(TPM_CHIP_FLAG_VIRTUAL, &chip->flags))
return;
sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
@@ -299,8 +300,9 @@ static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
struct attribute **i;
int rc;
- if (chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))
- return 0;
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags) ||
+ test_bit(TPM_CHIP_FLAG_VIRTUAL, &chip->flags))
+ return;
rc = __compat_only_sysfs_link_entry_to_kobj(
&chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
@@ -335,7 +337,7 @@ int tpm_chip_register(struct tpm_chip *chip)
int rc;
if (chip->ops->flags & TPM_OPS_AUTO_STARTUP) {
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
rc = tpm2_auto_startup(chip);
else
rc = tpm1_auto_startup(chip);
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 912ad30..3738c38 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -57,17 +57,14 @@ static int tpm_open(struct inode *inode, struct file *file)
container_of(inode->i_cdev, struct tpm_chip, cdev);
struct file_priv *priv;
- /* It's assured that the chip will be opened just once,
- * by the check of is_open variable, which is protected
- * by driver_lock. */
- if (test_and_set_bit(0, &chip->is_open)) {
+ if (test_and_set_bit(TPM_CHIP_FLAG_IS_OPEN, &chip->flags)) {
dev_dbg(&chip->dev, "Another process owns this TPM\n");
return -EBUSY;
}
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (priv == NULL) {
- clear_bit(0, &chip->is_open);
+ clear_bit(TPM_CHIP_FLAG_IS_OPEN, &chip->flags);
return -ENOMEM;
}
@@ -173,7 +170,7 @@ static int tpm_release(struct inode *inode, struct file *file)
flush_work(&priv->work);
file->private_data = NULL;
atomic_set(&priv->data_pending, 0);
- clear_bit(0, &priv->chip->is_open);
+ clear_bit(TPM_CHIP_FLAG_IS_OPEN, &priv->chip->flags);
kfree(priv);
return 0;
}
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a2688ac..6426be7 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -367,10 +367,10 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
goto out;
}
- if (chip->flags & TPM_CHIP_FLAG_IRQ)
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags))
goto out_recv;
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
stop = jiffies + tpm2_calc_ordinal_duration(chip, ordinal);
else
stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
@@ -503,10 +503,10 @@ int tpm_get_timeouts(struct tpm_chip *chip)
unsigned long old_timeout[4];
ssize_t rc;
- if (chip->flags & TPM_CHIP_FLAG_HAVE_TIMEOUTS)
+ if (test_bit(TPM_CHIP_FLAG_HAVE_TIMEOUTS, &chip->flags))
return 0;
- if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) {
/* Fixed timeouts for TPM2 */
chip->timeout_a = msecs_to_jiffies(TPM2_TIMEOUT_A);
chip->timeout_b = msecs_to_jiffies(TPM2_TIMEOUT_B);
@@ -519,7 +519,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
chip->duration[TPM_LONG] =
msecs_to_jiffies(TPM2_DURATION_LONG);
- chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
+ set_bit(TPM_CHIP_FLAG_HAVE_TIMEOUTS, &chip->flags);
return 0;
}
@@ -600,11 +600,11 @@ int tpm_get_timeouts(struct tpm_chip *chip)
chip->duration[TPM_SHORT] = HZ;
chip->duration[TPM_MEDIUM] *= 1000;
chip->duration[TPM_LONG] *= 1000;
- chip->duration_adjusted = true;
+ set_bit(TPM_CHIP_FLAG_DURATION_ADJUSTED, &chip->flags);
dev_info(&chip->dev, "Adjusting TPM timeout parameters.");
}
- chip->flags |= TPM_CHIP_FLAG_HAVE_TIMEOUTS;
+ set_bit(TPM_CHIP_FLAG_HAVE_TIMEOUTS, &chip->flags);
return 0;
}
EXPORT_SYMBOL_GPL(tpm_get_timeouts);
@@ -676,7 +676,7 @@ int tpm_is_tpm2(u32 chip_num)
if (chip == NULL)
return -ENODEV;
- rc = (chip->flags & TPM_CHIP_FLAG_TPM2) != 0;
+ rc = (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) != 0;
tpm_put_ops(chip);
@@ -703,7 +703,7 @@ int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
chip = tpm_chip_find_get(chip_num);
if (chip == NULL)
return -ENODEV;
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
else
rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
@@ -740,7 +740,7 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
if (chip == NULL)
return -ENODEV;
- if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) {
rc = tpm2_pcr_extend(chip, pcr_idx, hash);
tpm_put_ops(chip);
return rc;
@@ -890,7 +890,7 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout,
stop = jiffies + timeout;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) {
again:
timeout = stop - jiffies;
if ((long)timeout <= 0)
@@ -944,7 +944,7 @@ int tpm_pm_suspend(struct device *dev)
if (chip == NULL)
return -ENODEV;
- if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) {
tpm2_shutdown(chip, TPM2_SU_STATE);
return 0;
}
@@ -1036,7 +1036,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
if (chip == NULL)
return -ENODEV;
- if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) {
err = tpm2_get_random(chip, out, max);
tpm_put_ops(chip);
return err;
@@ -1081,7 +1081,7 @@ int tpm_seal_trusted(u32 chip_num, struct trusted_key_payload *payload,
int rc;
chip = tpm_chip_find_get(chip_num);
- if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
+ if (chip == NULL || !(test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)))
return -ENODEV;
rc = tpm2_seal_trusted(chip, payload, options);
@@ -1107,7 +1107,7 @@ int tpm_unseal_trusted(u32 chip_num, struct trusted_key_payload *payload,
int rc;
chip = tpm_chip_find_get(chip_num);
- if (chip == NULL || !(chip->flags & TPM_CHIP_FLAG_TPM2))
+ if (chip == NULL || !(test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)))
return -ENODEV;
rc = tpm2_unseal_trusted(chip, payload, options);
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 848ad65..21b3fde 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -244,7 +244,7 @@ static ssize_t durations_show(struct device *dev, struct device_attribute *attr,
jiffies_to_usecs(chip->duration[TPM_SHORT]),
jiffies_to_usecs(chip->duration[TPM_MEDIUM]),
jiffies_to_usecs(chip->duration[TPM_LONG]),
- chip->duration_adjusted
+ test_bit(TPM_CHIP_FLAG_DURATION_ADJUSTED, &chip->flags)
? "adjusted" : "original");
}
static DEVICE_ATTR_RO(durations);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1ae9768..a1fadfa 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -142,10 +142,12 @@ enum tpm2_startup_types {
#define TPM_PPI_VERSION_LEN 3
enum tpm_chip_flags {
- TPM_CHIP_FLAG_TPM2 = BIT(1),
- TPM_CHIP_FLAG_IRQ = BIT(2),
- TPM_CHIP_FLAG_VIRTUAL = BIT(3),
- TPM_CHIP_FLAG_HAVE_TIMEOUTS = BIT(4),
+ TPM_CHIP_FLAG_IS_OPEN = 0,
+ TPM_CHIP_FLAG_TPM2 = 1,
+ TPM_CHIP_FLAG_IRQ = 2,
+ TPM_CHIP_FLAG_VIRTUAL = 3,
+ TPM_CHIP_FLAG_HAVE_TIMEOUTS = 4,
+ TPM_CHIP_FLAG_DURATION_ADJUSTED = 5,
};
struct tpm_chip_seqops {
@@ -168,10 +170,9 @@ struct tpm_chip {
struct tpm_chip_seqops bin_log_seqops;
struct tpm_chip_seqops ascii_log_seqops;
- unsigned int flags;
+ unsigned long flags;
int dev_num; /* /dev/tpm# */
- unsigned long is_open; /* only one allowed */
struct mutex tpm_mutex; /* tpm is processing */
@@ -181,7 +182,6 @@ struct tpm_chip {
unsigned long timeout_d; /* jiffies */
bool timeout_adjusted;
unsigned long duration[3]; /* jiffies */
- bool duration_adjusted;
struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index da5b782..7abe8a0 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -946,7 +946,7 @@ int tpm2_probe(struct tpm_chip *chip)
return rc;
if (be16_to_cpu(cmd.header.out.tag) == TPM2_ST_NO_SESSIONS)
- chip->flags |= TPM_CHIP_FLAG_TPM2;
+ set_bit(TPM_CHIP_FLAG_TPM2, &chip->flags);
return 0;
}
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 65040d7..e37ebaa 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -429,7 +429,7 @@ static int crb_acpi_add(struct acpi_device *device)
dev_set_drvdata(&chip->dev, priv);
chip->acpi_dev_handle = device->handle;
- chip->flags = TPM_CHIP_FLAG_TPM2;
+ set_bit(TPM_CHIP_FLAG_TPM2, &chip->flags);
rc = crb_cmd_ready(dev, priv);
if (rc)
diff --git a/drivers/char/tpm/tpm_i2c_nuvoton.c b/drivers/char/tpm/tpm_i2c_nuvoton.c
index e3a9155..f4e25b5 100644
--- a/drivers/char/tpm/tpm_i2c_nuvoton.c
+++ b/drivers/char/tpm/tpm_i2c_nuvoton.c
@@ -180,7 +180,7 @@ static bool i2c_nuvoton_check_status(struct tpm_chip *chip, u8 mask, u8 value)
static int i2c_nuvoton_wait_for_stat(struct tpm_chip *chip, u8 mask, u8 value,
u32 timeout, wait_queue_head_t *queue)
{
- if ((chip->flags & TPM_CHIP_FLAG_IRQ) && queue) {
+ if ((test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) && queue) {
s32 rc;
struct priv_data *priv = dev_get_drvdata(&chip->dev);
unsigned int cur_intrs = priv->intrs;
@@ -552,10 +552,10 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
of_id = of_match_device(dev->driver->of_match_table, dev);
if (of_id && of_id->data == OF_IS_TPM2)
- chip->flags |= TPM_CHIP_FLAG_TPM2;
+ set_bit(TPM_CHIP_FLAG_TPM2, &chip->flags);
} else
if (id->driver_data == I2C_IS_TPM2)
- chip->flags |= TPM_CHIP_FLAG_TPM2;
+ set_bit(TPM_CHIP_FLAG_TPM2, &chip->flags);
init_waitqueue_head(&priv->read_queue);
@@ -585,7 +585,7 @@ static int i2c_nuvoton_probe(struct i2c_client *client,
__func__, priv->irq);
priv->irq = 0;
} else {
- chip->flags |= TPM_CHIP_FLAG_IRQ;
+ set_bit(TPM_CHIP_FLAG_IRQ, &chip->flags);
/* Clear any pending interrupt */
i2c_nuvoton_ready(chip);
/* - wait for TPM_STS==0xA0 (stsValid, commandReady) */
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 7993678..e6af2a0 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -105,7 +105,7 @@ static int request_locality(struct tpm_chip *chip, int l)
stop = jiffies + chip->timeout_a;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) {
again:
timeout = stop - jiffies;
if ((long)timeout <= 0)
@@ -346,7 +346,7 @@ static void disable_interrupts(struct tpm_chip *chip)
devm_free_irq(chip->dev.parent, priv->irq, chip);
priv->irq = 0;
- chip->flags &= ~TPM_CHIP_FLAG_IRQ;
+ clear_bit(TPM_CHIP_FLAG_IRQ, &chip->flags);
}
/*
@@ -370,10 +370,10 @@ static int tpm_tis_send_main(struct tpm_chip *chip, u8 *buf, size_t len)
if (rc < 0)
goto out_err;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) {
ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
dur = tpm2_calc_ordinal_duration(chip, ordinal);
else
dur = tpm_calc_ordinal_duration(chip, ordinal);
@@ -397,16 +397,16 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
int rc, irq;
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
- if (!(chip->flags & TPM_CHIP_FLAG_IRQ) || priv->irq_tested)
+ if (!(test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)) || priv->irq_tested)
return tpm_tis_send_main(chip, buf, len);
/* Verify receipt of the expected IRQ */
irq = priv->irq;
priv->irq = 0;
- chip->flags &= ~TPM_CHIP_FLAG_IRQ;
+ clear_bit(TPM_CHIP_FLAG_IRQ, &chip->flags);
rc = tpm_tis_send_main(chip, buf, len);
priv->irq = irq;
- chip->flags |= TPM_CHIP_FLAG_IRQ;
+ set_bit(TPM_CHIP_FLAG_IRQ, &chip->flags);
if (!priv->irq_tested)
msleep(1);
if (!priv->irq_tested)
@@ -549,7 +549,7 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
u32 cap2;
cap_t cap;
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ if (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags))
return tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
else
return tpm_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc);
@@ -611,7 +611,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
/* tpm_tis_send will either confirm the interrupt is working or it
* will call disable_irq which undoes all of the above.
*/
- if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
+ if (!(test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags))) {
rc = tpm_tis_write8(priv, original_int_vec,
TPM_INT_VECTOR(priv->locality));
if (rc < 0)
@@ -737,7 +737,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
goto out_err;
dev_info(dev, "%s TPM (device-id 0x%X, rev-id %d)\n",
- (chip->flags & TPM_CHIP_FLAG_TPM2) ? "2.0" : "1.2",
+ (test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)) ? "2.0" : "1.2",
vendor >> 16, rid);
if (!(priv->flags & TPM_TIS_ITPM_POSSIBLE)) {
@@ -794,7 +794,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
if (irq) {
tpm_tis_probe_irq_single(chip, intmask, IRQF_SHARED,
irq);
- if (!(chip->flags & TPM_CHIP_FLAG_IRQ))
+ if (!(test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags)))
dev_err(&chip->dev, FW_BUG
"TPM interrupt not working, polling instead\n");
} else {
@@ -839,7 +839,7 @@ int tpm_tis_resume(struct device *dev)
struct tpm_chip *chip = dev_get_drvdata(dev);
int ret;
- if (chip->flags & TPM_CHIP_FLAG_IRQ)
+ if (test_bit(TPM_CHIP_FLAG_IRQ, &chip->flags))
tpm_tis_reenable_interrupts(chip);
ret = tpm_pm_resume(dev);
@@ -849,7 +849,7 @@ int tpm_tis_resume(struct device *dev)
/* TPM 1.2 requires self-test on resume. This function actually returns
* an error code but for unknown reason it isn't handled.
*/
- if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+ if (!(test_bit(TPM_CHIP_FLAG_TPM2, &chip->flags)))
tpm_do_selftest(chip);
return 0;
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 3d6f6ca..b0cb284 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -482,7 +482,7 @@ static struct file *vtpm_proxy_create_device(
vtpm_proxy_fops_open(file);
if (proxy_dev->flags & VTPM_PROXY_FLAG_TPM2)
- proxy_dev->chip->flags |= TPM_CHIP_FLAG_TPM2;
+ set_bit(TPM_CHIP_FLAG_TPM2, &proxy_dev->chip->flags);
vtpm_proxy_work_start(proxy_dev);
--
2.9.3
------------------------------------------------------------------------------
^ permalink raw reply related
* Re: [PATCH] hpsa: correct logical resets
From: Martin K. Petersen @ 2016-11-14 23:44 UTC (permalink / raw)
To: Don Brace
Cc: jejb, john.hall, Kevin.Barnett, Mahesh.Rajashekhara,
bader.alisaleh, hch, scott.teel, Viswas.G, Justin.Lindley,
scott.benesh, elliott, POSWALD, linux-scsi
In-Reply-To: <147881417483.804.10756927186886954941.stgit@brunhilda>
>>>>> "Don" == Don Brace <don.brace@microsemi.com> writes:
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 0b6eb5a..a296537 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -300,6 +300,10 @@ static bool hpsa_cmd_dev_match(struct ctlr_info *h, struct CommandList *c,
struct hpsa_scsi_dev_t *dev,
unsigned char *scsi3addr);
+static int wait_for_device_to_become_ready(struct ctlr_info *h,
+ unsigned char lunaddr[],
+ int reply_queue);
+
static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev)
{
unsigned long *priv = shost_priv(sdev->host);
Wouldn't it be nicer to put this with the rest of the function
prototypes at the beginning of the file?
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply related
* [PATCH v1 3/3] powerpc: fix node_possible_map limitations
From: Balbir Singh @ 2016-11-14 23:44 UTC (permalink / raw)
To: linux-mm; +Cc: linuxppc-dev, mpe, akpm, tj, Balbir Singh
In-Reply-To: <1479167045-28136-1-git-send-email-bsingharora@gmail.com>
We've fixed the memory hotplug issue with memcg, hence
this work around should not be required.
Fixes: commit 3af229f2071f
("powerpc/numa: Reset node_possible_map to only node_online_map")
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/mm/numa.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index a51c188..ca8c2ab 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -916,13 +916,6 @@ void __init initmem_init(void)
memblock_dump_all();
- /*
- * Reduce the possible NUMA nodes to the online NUMA nodes,
- * since we do not support node hotplug. This ensures that we
- * lower the maximum NUMA node ID to what is actually present.
- */
- nodes_and(node_possible_map, node_possible_map, node_online_map);
-
for_each_online_node(nid) {
unsigned long start_pfn, end_pfn;
--
2.5.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v1 3/3] powerpc: fix node_possible_map limitations
From: Balbir Singh @ 2016-11-14 23:44 UTC (permalink / raw)
To: linux-mm; +Cc: linuxppc-dev, mpe, akpm, tj, Balbir Singh
In-Reply-To: <1479167045-28136-1-git-send-email-bsingharora@gmail.com>
We've fixed the memory hotplug issue with memcg, hence
this work around should not be required.
Fixes: commit 3af229f2071f
("powerpc/numa: Reset node_possible_map to only node_online_map")
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/mm/numa.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index a51c188..ca8c2ab 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -916,13 +916,6 @@ void __init initmem_init(void)
memblock_dump_all();
- /*
- * Reduce the possible NUMA nodes to the online NUMA nodes,
- * since we do not support node hotplug. This ensures that we
- * lower the maximum NUMA node ID to what is actually present.
- */
- nodes_and(node_possible_map, node_possible_map, node_online_map);
-
for_each_online_node(nid) {
unsigned long start_pfn, end_pfn;
--
2.5.5
^ permalink raw reply related
* [PATCH v1 2/3] Move from all possible nodes to online nodes
From: Balbir Singh @ 2016-11-14 23:44 UTC (permalink / raw)
To: linux-mm; +Cc: linuxppc-dev, mpe, akpm, tj, Balbir Singh
In-Reply-To: <1479167045-28136-1-git-send-email-bsingharora@gmail.com>
Move routines that do operations on all nodes to
just the online nodes. Most of the changes are
very obvious (like the ones related to soft limit tree
per node)
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
mm/memcontrol.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 5585fce..cc49fa2 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -497,7 +497,7 @@ static void mem_cgroup_remove_from_trees(struct mem_cgroup *memcg)
struct mem_cgroup_per_node *mz;
int nid;
- for_each_node(nid) {
+ for_each_online_node(nid) {
mz = mem_cgroup_nodeinfo(memcg, nid);
mctz = soft_limit_tree_node(nid);
mem_cgroup_remove_exceeded(mz, mctz);
@@ -895,7 +895,7 @@ static void invalidate_reclaim_iterators(struct mem_cgroup *dead_memcg)
int i;
while ((memcg = parent_mem_cgroup(memcg))) {
- for_each_node(nid) {
+ for_each_online_node(nid) {
mz = mem_cgroup_nodeinfo(memcg, nid);
for (i = 0; i <= DEF_PRIORITY; i++) {
iter = &mz->iter[i];
@@ -4146,7 +4146,7 @@ static void mem_cgroup_free(struct mem_cgroup *memcg)
int node;
memcg_wb_domain_exit(memcg);
- for_each_node(node)
+ for_each_online_node(node)
free_mem_cgroup_per_node_info(memcg, node);
free_percpu(memcg->stat);
kfree(memcg);
@@ -4175,7 +4175,7 @@ static struct mem_cgroup *mem_cgroup_alloc(void)
if (!memcg->stat)
goto fail;
- for_each_node(node)
+ for_each_online_node(node)
if (alloc_mem_cgroup_per_node_info(memcg, node))
goto fail;
@@ -5774,11 +5774,21 @@ __setup("cgroup.memory=", cgroup_memory);
static void memcg_node_offline(int node)
{
struct mem_cgroup *memcg;
+ struct mem_cgroup_tree_per_node *rtpn;
+ struct mem_cgroup_tree_per_node *mctz;
+ struct mem_cgroup_per_node *mz;
if (node < 0)
return;
+ rtpn = soft_limit_tree.rb_tree_per_node[node];
+ kfree(rtpn);
+
for_each_mem_cgroup(memcg) {
+ mz = mem_cgroup_nodeinfo(memcg, node);
+ mctz = soft_limit_tree_node(node);
+ mem_cgroup_remove_exceeded(mz, mctz);
+
free_mem_cgroup_per_node_info(memcg, node);
mem_cgroup_may_update_nodemask(memcg);
}
@@ -5787,10 +5797,18 @@ static void memcg_node_offline(int node)
static void memcg_node_online(int node)
{
struct mem_cgroup *memcg;
+ struct mem_cgroup_tree_per_node *rtpn;
if (node < 0)
return;
+ rtpn = kzalloc_node(sizeof(*rtpn), GFP_KERNEL,
+ node_online(node) ? node : NUMA_NO_NODE);
+
+ rtpn->rb_root = RB_ROOT;
+ spin_lock_init(&rtpn->lock);
+ soft_limit_tree.rb_tree_per_node[node] = rtpn;
+
for_each_mem_cgroup(memcg) {
alloc_mem_cgroup_per_node_info(memcg, node);
mem_cgroup_may_update_nodemask(memcg);
@@ -5854,7 +5872,7 @@ static int __init mem_cgroup_init(void)
INIT_WORK(&per_cpu_ptr(&memcg_stock, cpu)->work,
drain_local_stock);
- for_each_node(node) {
+ for_each_online_node(node) {
struct mem_cgroup_tree_per_node *rtpn;
rtpn = kzalloc_node(sizeof(*rtpn), GFP_KERNEL,
--
2.5.5
^ permalink raw reply related
* [PATCH v1 2/3] Move from all possible nodes to online nodes
From: Balbir Singh @ 2016-11-14 23:44 UTC (permalink / raw)
To: linux-mm; +Cc: linuxppc-dev, mpe, akpm, tj, Balbir Singh
In-Reply-To: <1479167045-28136-1-git-send-email-bsingharora@gmail.com>
Move routines that do operations on all nodes to
just the online nodes. Most of the changes are
very obvious (like the ones related to soft limit tree
per node)
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
mm/memcontrol.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 5585fce..cc49fa2 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -497,7 +497,7 @@ static void mem_cgroup_remove_from_trees(struct mem_cgroup *memcg)
struct mem_cgroup_per_node *mz;
int nid;
- for_each_node(nid) {
+ for_each_online_node(nid) {
mz = mem_cgroup_nodeinfo(memcg, nid);
mctz = soft_limit_tree_node(nid);
mem_cgroup_remove_exceeded(mz, mctz);
@@ -895,7 +895,7 @@ static void invalidate_reclaim_iterators(struct mem_cgroup *dead_memcg)
int i;
while ((memcg = parent_mem_cgroup(memcg))) {
- for_each_node(nid) {
+ for_each_online_node(nid) {
mz = mem_cgroup_nodeinfo(memcg, nid);
for (i = 0; i <= DEF_PRIORITY; i++) {
iter = &mz->iter[i];
@@ -4146,7 +4146,7 @@ static void mem_cgroup_free(struct mem_cgroup *memcg)
int node;
memcg_wb_domain_exit(memcg);
- for_each_node(node)
+ for_each_online_node(node)
free_mem_cgroup_per_node_info(memcg, node);
free_percpu(memcg->stat);
kfree(memcg);
@@ -4175,7 +4175,7 @@ static struct mem_cgroup *mem_cgroup_alloc(void)
if (!memcg->stat)
goto fail;
- for_each_node(node)
+ for_each_online_node(node)
if (alloc_mem_cgroup_per_node_info(memcg, node))
goto fail;
@@ -5774,11 +5774,21 @@ __setup("cgroup.memory=", cgroup_memory);
static void memcg_node_offline(int node)
{
struct mem_cgroup *memcg;
+ struct mem_cgroup_tree_per_node *rtpn;
+ struct mem_cgroup_tree_per_node *mctz;
+ struct mem_cgroup_per_node *mz;
if (node < 0)
return;
+ rtpn = soft_limit_tree.rb_tree_per_node[node];
+ kfree(rtpn);
+
for_each_mem_cgroup(memcg) {
+ mz = mem_cgroup_nodeinfo(memcg, node);
+ mctz = soft_limit_tree_node(node);
+ mem_cgroup_remove_exceeded(mz, mctz);
+
free_mem_cgroup_per_node_info(memcg, node);
mem_cgroup_may_update_nodemask(memcg);
}
@@ -5787,10 +5797,18 @@ static void memcg_node_offline(int node)
static void memcg_node_online(int node)
{
struct mem_cgroup *memcg;
+ struct mem_cgroup_tree_per_node *rtpn;
if (node < 0)
return;
+ rtpn = kzalloc_node(sizeof(*rtpn), GFP_KERNEL,
+ node_online(node) ? node : NUMA_NO_NODE);
+
+ rtpn->rb_root = RB_ROOT;
+ spin_lock_init(&rtpn->lock);
+ soft_limit_tree.rb_tree_per_node[node] = rtpn;
+
for_each_mem_cgroup(memcg) {
alloc_mem_cgroup_per_node_info(memcg, node);
mem_cgroup_may_update_nodemask(memcg);
@@ -5854,7 +5872,7 @@ static int __init mem_cgroup_init(void)
INIT_WORK(&per_cpu_ptr(&memcg_stock, cpu)->work,
drain_local_stock);
- for_each_node(node) {
+ for_each_online_node(node) {
struct mem_cgroup_tree_per_node *rtpn;
rtpn = kzalloc_node(sizeof(*rtpn), GFP_KERNEL,
--
2.5.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v1 1/3] Add basic infrastructure for memcg hotplug support
From: Balbir Singh @ 2016-11-14 23:44 UTC (permalink / raw)
To: linux-mm; +Cc: linuxppc-dev, mpe, akpm, tj, Balbir Singh
In-Reply-To: <1479167045-28136-1-git-send-email-bsingharora@gmail.com>
The lack of hotplug support makes us allocate all memory
upfront for per node data structures. With large number
of cgroups this can be an overhead. PPC64 actually limits
n_possible nodes to n_online to avoid some of this overhead.
This patch adds the basic notifiers to listen to hotplug
events and does the allocation and free of those structures
per cgroup. We walk every cgroup per event, its a trade-off
of allocating upfront vs allocating on demand and freeing
on offline.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
mm/memcontrol.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 60 insertions(+), 8 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 91dfc7c..5585fce 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -63,6 +63,7 @@
#include <linux/lockdep.h>
#include <linux/file.h>
#include <linux/tracehook.h>
+#include <linux/memory.h>
#include "internal.h"
#include <net/sock.h>
#include <net/ip.h>
@@ -1342,6 +1343,10 @@ int mem_cgroup_select_victim_node(struct mem_cgroup *memcg)
{
return 0;
}
+
+static void mem_cgroup_may_update_nodemask(struct mem_cgroup *memcg)
+{
+}
#endif
static int mem_cgroup_soft_reclaim(struct mem_cgroup *root_memcg,
@@ -4115,14 +4120,7 @@ static int alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
{
struct mem_cgroup_per_node *pn;
int tmp = node;
- /*
- * This routine is called against possible nodes.
- * But it's BUG to call kmalloc() against offline node.
- *
- * TODO: this routine can waste much memory for nodes which will
- * never be onlined. It's better to use memory hotplug callback
- * function.
- */
+
if (!node_state(node, N_NORMAL_MEMORY))
tmp = -1;
pn = kzalloc_node(sizeof(*pn), GFP_KERNEL, tmp);
@@ -5773,6 +5771,59 @@ static int __init cgroup_memory(char *s)
}
__setup("cgroup.memory=", cgroup_memory);
+static void memcg_node_offline(int node)
+{
+ struct mem_cgroup *memcg;
+
+ if (node < 0)
+ return;
+
+ for_each_mem_cgroup(memcg) {
+ free_mem_cgroup_per_node_info(memcg, node);
+ mem_cgroup_may_update_nodemask(memcg);
+ }
+}
+
+static void memcg_node_online(int node)
+{
+ struct mem_cgroup *memcg;
+
+ if (node < 0)
+ return;
+
+ for_each_mem_cgroup(memcg) {
+ alloc_mem_cgroup_per_node_info(memcg, node);
+ mem_cgroup_may_update_nodemask(memcg);
+ }
+}
+
+static int memcg_memory_hotplug_callback(struct notifier_block *self,
+ unsigned long action, void *arg)
+{
+ struct memory_notify *marg = arg;
+ int node = marg->status_change_nid;
+
+ switch (action) {
+ case MEM_GOING_OFFLINE:
+ case MEM_CANCEL_ONLINE:
+ memcg_node_offline(node);
+ break;
+ case MEM_GOING_ONLINE:
+ case MEM_CANCEL_OFFLINE:
+ memcg_node_online(node);
+ break;
+ case MEM_ONLINE:
+ case MEM_OFFLINE:
+ break;
+ }
+ return NOTIFY_OK;
+}
+
+static struct notifier_block memcg_memory_hotplug_nb __meminitdata = {
+ .notifier_call = memcg_memory_hotplug_callback,
+ .priority = IPC_CALLBACK_PRI,
+};
+
/*
* subsys_initcall() for memory controller.
*
@@ -5797,6 +5848,7 @@ static int __init mem_cgroup_init(void)
#endif
hotcpu_notifier(memcg_cpu_hotplug_callback, 0);
+ register_hotmemory_notifier(&memcg_memory_hotplug_nb);
for_each_possible_cpu(cpu)
INIT_WORK(&per_cpu_ptr(&memcg_stock, cpu)->work,
--
2.5.5
^ permalink raw reply related
* [PATCH v1 1/3] Add basic infrastructure for memcg hotplug support
From: Balbir Singh @ 2016-11-14 23:44 UTC (permalink / raw)
To: linux-mm; +Cc: linuxppc-dev, mpe, akpm, tj, Balbir Singh
In-Reply-To: <1479167045-28136-1-git-send-email-bsingharora@gmail.com>
The lack of hotplug support makes us allocate all memory
upfront for per node data structures. With large number
of cgroups this can be an overhead. PPC64 actually limits
n_possible nodes to n_online to avoid some of this overhead.
This patch adds the basic notifiers to listen to hotplug
events and does the allocation and free of those structures
per cgroup. We walk every cgroup per event, its a trade-off
of allocating upfront vs allocating on demand and freeing
on offline.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
mm/memcontrol.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 60 insertions(+), 8 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 91dfc7c..5585fce 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -63,6 +63,7 @@
#include <linux/lockdep.h>
#include <linux/file.h>
#include <linux/tracehook.h>
+#include <linux/memory.h>
#include "internal.h"
#include <net/sock.h>
#include <net/ip.h>
@@ -1342,6 +1343,10 @@ int mem_cgroup_select_victim_node(struct mem_cgroup *memcg)
{
return 0;
}
+
+static void mem_cgroup_may_update_nodemask(struct mem_cgroup *memcg)
+{
+}
#endif
static int mem_cgroup_soft_reclaim(struct mem_cgroup *root_memcg,
@@ -4115,14 +4120,7 @@ static int alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
{
struct mem_cgroup_per_node *pn;
int tmp = node;
- /*
- * This routine is called against possible nodes.
- * But it's BUG to call kmalloc() against offline node.
- *
- * TODO: this routine can waste much memory for nodes which will
- * never be onlined. It's better to use memory hotplug callback
- * function.
- */
+
if (!node_state(node, N_NORMAL_MEMORY))
tmp = -1;
pn = kzalloc_node(sizeof(*pn), GFP_KERNEL, tmp);
@@ -5773,6 +5771,59 @@ static int __init cgroup_memory(char *s)
}
__setup("cgroup.memory=", cgroup_memory);
+static void memcg_node_offline(int node)
+{
+ struct mem_cgroup *memcg;
+
+ if (node < 0)
+ return;
+
+ for_each_mem_cgroup(memcg) {
+ free_mem_cgroup_per_node_info(memcg, node);
+ mem_cgroup_may_update_nodemask(memcg);
+ }
+}
+
+static void memcg_node_online(int node)
+{
+ struct mem_cgroup *memcg;
+
+ if (node < 0)
+ return;
+
+ for_each_mem_cgroup(memcg) {
+ alloc_mem_cgroup_per_node_info(memcg, node);
+ mem_cgroup_may_update_nodemask(memcg);
+ }
+}
+
+static int memcg_memory_hotplug_callback(struct notifier_block *self,
+ unsigned long action, void *arg)
+{
+ struct memory_notify *marg = arg;
+ int node = marg->status_change_nid;
+
+ switch (action) {
+ case MEM_GOING_OFFLINE:
+ case MEM_CANCEL_ONLINE:
+ memcg_node_offline(node);
+ break;
+ case MEM_GOING_ONLINE:
+ case MEM_CANCEL_OFFLINE:
+ memcg_node_online(node);
+ break;
+ case MEM_ONLINE:
+ case MEM_OFFLINE:
+ break;
+ }
+ return NOTIFY_OK;
+}
+
+static struct notifier_block memcg_memory_hotplug_nb __meminitdata = {
+ .notifier_call = memcg_memory_hotplug_callback,
+ .priority = IPC_CALLBACK_PRI,
+};
+
/*
* subsys_initcall() for memory controller.
*
@@ -5797,6 +5848,7 @@ static int __init mem_cgroup_init(void)
#endif
hotcpu_notifier(memcg_cpu_hotplug_callback, 0);
+ register_hotmemory_notifier(&memcg_memory_hotplug_nb);
for_each_possible_cpu(cpu)
INIT_WORK(&per_cpu_ptr(&memcg_stock, cpu)->work,
--
2.5.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [v1 0/3] Support memory cgroup hotplug
From: Balbir Singh @ 2016-11-14 23:44 UTC (permalink / raw)
To: linux-mm; +Cc: linuxppc-dev, mpe, akpm, tj, Balbir Singh
In the absence of hotplug we use extra memory proportional to
(possible_nodes - online_nodes) * number_of_cgroups. PPC64 has a patch
to disable large consumption with large number of cgroups. This patch
adds hotplug support to memory cgroups and reverts the commit that
limited possible nodes to online nodes.
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
I've tested this patches under a VM with two nodes and movable
nodes enabled. I've offlined nodes and checked that the system
and cgroups with tasks deep in the hierarchy continue to work
fine.
Balbir Singh (3):
Add basic infrastructure for memcg hotplug support
Move from all possible nodes to online nodes
powerpc: fix node_possible_map limitations
arch/powerpc/mm/numa.c | 7 ----
mm/memcontrol.c | 96 +++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 83 insertions(+), 20 deletions(-)
--
2.5.5
^ permalink raw reply
* [v1 0/3] Support memory cgroup hotplug
From: Balbir Singh @ 2016-11-14 23:44 UTC (permalink / raw)
To: linux-mm; +Cc: linuxppc-dev, mpe, akpm, tj, Balbir Singh
In the absence of hotplug we use extra memory proportional to
(possible_nodes - online_nodes) * number_of_cgroups. PPC64 has a patch
to disable large consumption with large number of cgroups. This patch
adds hotplug support to memory cgroups and reverts the commit that
limited possible nodes to online nodes.
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
I've tested this patches under a VM with two nodes and movable
nodes enabled. I've offlined nodes and checked that the system
and cgroups with tasks deep in the hierarchy continue to work
fine.
Balbir Singh (3):
Add basic infrastructure for memcg hotplug support
Move from all possible nodes to online nodes
powerpc: fix node_possible_map limitations
arch/powerpc/mm/numa.c | 7 ----
mm/memcontrol.c | 96 +++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 83 insertions(+), 20 deletions(-)
--
2.5.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v6 5/9] tpm: have event log use the tpm_chip
From: Jarkko Sakkinen @ 2016-11-14 23:44 UTC (permalink / raw)
To: Nayna Jain
Cc: tpmdd-devel, peterhuewe, tpmdd, jgunthorpe, linux-kernel,
linux-security-module
In-Reply-To: <1479117656-12403-6-git-send-email-nayna@linux.vnet.ibm.com>
On Mon, Nov 14, 2016 at 05:00:52AM -0500, Nayna Jain wrote:
> Move the backing memory for the event log into tpm_chip and push
> the tpm_chip into read_log. This optimizes read_log processing by
> only doing it once and prepares things for the next patches in the
> series which require the tpm_chip to locate the event log via
> ACPI and OF handles instead of searching.
>
> This is straightfoward except for the issue of passing a kref through
> i_private with securityfs. Since securityfs_remove does not have any
> removal fencing like sysfs we use the inode lock to safely get a
> kref on the tpm_chip.
>
> Suggested-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
> ---
> drivers/char/tpm/tpm-chip.c | 3 +-
> drivers/char/tpm/tpm.h | 11 ++++++
> drivers/char/tpm/tpm_acpi.c | 15 +++++--
> drivers/char/tpm/tpm_eventlog.c | 88 ++++++++++++++++++++++++++---------------
> drivers/char/tpm/tpm_eventlog.h | 2 +-
> drivers/char/tpm/tpm_of.c | 4 +-
> 6 files changed, 85 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 250a651..3f27753 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -127,6 +127,7 @@ static void tpm_dev_release(struct device *dev)
> idr_remove(&dev_nums_idr, chip->dev_num);
> mutex_unlock(&idr_lock);
>
> + kfree(chip->log.bios_event_log);
> kfree(chip);
> }
>
> @@ -345,7 +346,7 @@ int tpm_chip_register(struct tpm_chip *chip)
> tpm_sysfs_add_device(chip);
>
> rc = tpm_bios_log_setup(chip);
> - if (rc)
> + if (rc == -ENODEV)
> return rc;
>
> tpm_add_ppi(chip);
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 9d69580..1ae9768 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -35,6 +35,8 @@
> #include <linux/cdev.h>
> #include <linux/highmem.h>
>
> +#include "tpm_eventlog.h"
> +
> enum tpm_const {
> TPM_MINOR = 224, /* officially assigned */
> TPM_BUFSIZE = 4096,
> @@ -146,6 +148,11 @@ enum tpm_chip_flags {
> TPM_CHIP_FLAG_HAVE_TIMEOUTS = BIT(4),
> };
>
> +struct tpm_chip_seqops {
> + struct tpm_chip *chip;
> + const struct seq_operations *seqops;
> +};
> +
> struct tpm_chip {
> struct device dev;
> struct cdev cdev;
> @@ -157,6 +164,10 @@ struct tpm_chip {
> struct rw_semaphore ops_sem;
> const struct tpm_class_ops *ops;
>
> + struct tpm_bios_log log;
> + struct tpm_chip_seqops bin_log_seqops;
> + struct tpm_chip_seqops ascii_log_seqops;
> +
> unsigned int flags;
>
> int dev_num; /* /dev/tpm# */
> diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
> index 565a947..01dfb35 100644
> --- a/drivers/char/tpm/tpm_acpi.c
> +++ b/drivers/char/tpm/tpm_acpi.c
> @@ -9,7 +9,7 @@
> *
> * Maintained by: <tpmdd-devel@lists.sourceforge.net>
> *
> - * Access to the eventlog extended by the TCG BIOS of PC platform
> + * Access to the event log extended by the TCG BIOS of PC platform
> *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of the GNU General Public License
> @@ -45,13 +45,15 @@ struct acpi_tcpa {
> };
>
> /* read binary bios log */
> -int read_log(struct tpm_bios_log *log)
> +int read_log(struct tpm_chip *chip)
> {
> struct acpi_tcpa *buff;
> acpi_status status;
> void __iomem *virt;
> u64 len, start;
> + struct tpm_bios_log *log;
>
> + log = &chip->log;
> if (log->bios_event_log != NULL) {
> printk(KERN_ERR
> "%s: ERROR - Eventlog already initialized\n",
> @@ -97,13 +99,18 @@ int read_log(struct tpm_bios_log *log)
>
> virt = acpi_os_map_iomem(start, len);
> if (!virt) {
> - kfree(log->bios_event_log);
> printk("%s: ERROR - Unable to map memory\n", __func__);
> - return -EIO;
> + goto err;
> }
>
> memcpy_fromio(log->bios_event_log, virt, len);
>
> acpi_os_unmap_iomem(virt, len);
> return 0;
> +
> +err:
> + kfree(log->bios_event_log);
> + log->bios_event_log = NULL;
> + return -EIO;
> +
> }
> diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
> index 57ac862..f8c42fe 100644
> --- a/drivers/char/tpm/tpm_eventlog.c
> +++ b/drivers/char/tpm/tpm_eventlog.c
> @@ -11,7 +11,7 @@
> *
> * Maintained by: <tpmdd-devel@lists.sourceforge.net>
> *
> - * Access to the eventlog created by a system's firmware / BIOS
> + * Access to the event log created by a system's firmware / BIOS
> *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of the GNU General Public License
> @@ -73,7 +73,8 @@ static const char* tcpa_pc_event_id_strings[] = {
> static void *tpm_bios_measurements_start(struct seq_file *m, loff_t *pos)
> {
> loff_t i;
> - struct tpm_bios_log *log = m->private;
> + struct tpm_chip *chip = m->private;
> + struct tpm_bios_log *log = &chip->log;
> void *addr = log->bios_event_log;
> void *limit = log->bios_event_log_end;
> struct tcpa_event *event;
> @@ -120,7 +121,8 @@ static void *tpm_bios_measurements_next(struct seq_file *m, void *v,
> loff_t *pos)
> {
> struct tcpa_event *event = v;
> - struct tpm_bios_log *log = m->private;
> + struct tpm_chip *chip = m->private;
> + struct tpm_bios_log *log = &chip->log;
> void *limit = log->bios_event_log_end;
> u32 converted_event_size;
> u32 converted_event_type;
> @@ -261,13 +263,10 @@ static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
> static int tpm_bios_measurements_release(struct inode *inode,
> struct file *file)
> {
> - struct seq_file *seq = file->private_data;
> - struct tpm_bios_log *log = seq->private;
> + struct seq_file *seq = (struct seq_file *)file->private_data;
> + struct tpm_chip *chip = (struct tpm_chip *)seq->private;
>
> - if (log) {
> - kfree(log->bios_event_log);
> - kfree(log);
> - }
> + put_device(&chip->dev);
>
> return seq_release(inode, file);
> }
> @@ -323,33 +322,30 @@ static int tpm_bios_measurements_open(struct inode *inode,
> struct file *file)
> {
> int err;
> - struct tpm_bios_log *log;
> struct seq_file *seq;
> - const struct seq_operations *seqops =
> - (const struct seq_operations *)inode->i_private;
> -
> - log = kzalloc(sizeof(struct tpm_bios_log), GFP_KERNEL);
> - if (!log)
> - return -ENOMEM;
> -
> - if ((err = read_log(log)))
> - goto out_free;
> + struct tpm_chip_seqops *chip_seqops;
> + const struct seq_operations *seqops;
> + struct tpm_chip *chip;
> +
> + inode_lock(inode);
> + if (!inode->i_private) {
> + inode_unlock(inode);
> + return -ENODEV;
> + }
> + chip_seqops = (struct tpm_chip_seqops *)inode->i_private;
> + seqops = chip_seqops->seqops;
> + chip = chip_seqops->chip;
> + get_device(&chip->dev);
> + inode_unlock(inode);
>
> /* now register seq file */
> err = seq_open(file, seqops);
> if (!err) {
> seq = file->private_data;
> - seq->private = log;
> - } else {
> - goto out_free;
> + seq->private = chip;
> }
>
> -out:
> return err;
> -out_free:
> - kfree(log->bios_event_log);
> - kfree(log);
> - goto out;
> }
>
> static const struct file_operations tpm_bios_measurements_ops = {
> @@ -372,29 +368,47 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
> {
> const char *name = dev_name(&chip->dev);
> unsigned int cnt;
> + int rc = 0;
>
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> return 0;
>
> + rc = read_log(chip);
> + /*
> + * read_log failure means event log is not supported except for ENOMEM.
> + */
> + if (rc < 0) {
> + if (rc == -ENOMEM)
> + return -ENODEV;
> + else
> + return rc;
> + }
> +
> cnt = 0;
> chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
> if (is_bad(chip->bios_dir[cnt]))
> goto err;
> cnt++;
>
> + chip->bin_log_seqops.chip = chip;
> + chip->bin_log_seqops.seqops = &tpm_binary_b_measurements_seqops;
> +
> chip->bios_dir[cnt] =
> securityfs_create_file("binary_bios_measurements",
> 0440, chip->bios_dir[0],
> - (void *)&tpm_binary_b_measurements_seqops,
> + (void *)&chip->bin_log_seqops,
> &tpm_bios_measurements_ops);
> if (is_bad(chip->bios_dir[cnt]))
> goto err;
> cnt++;
>
> + chip->ascii_log_seqops.chip = chip;
> + chip->ascii_log_seqops.seqops = &tpm_ascii_b_measurements_seqops;
> +
> chip->bios_dir[cnt] =
> securityfs_create_file("ascii_bios_measurements",
> 0440, chip->bios_dir[0],
> - (void *)&tpm_ascii_b_measurements_seqops,
> + (void *)&chip->ascii_log_seqops,
> &tpm_bios_measurements_ops);
> if (is_bad(chip->bios_dir[cnt]))
> goto err;
> @@ -411,7 +425,19 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
> void tpm_bios_log_teardown(struct tpm_chip *chip)
> {
> int i;
> -
> - for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--)
> + struct inode *inode;
> +
> + /* securityfs_remove currently doesn't take care of handling sync
> + * between removal and opening of pseudo files. To handle this, a
> + * workaround is added by making i_private = NULL here during removal
> + * and to check it during open(), both within inode_lock()/unlock().
> + * This design ensures that open() either safely gets kref or fails.
> + */
> + for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--) {
> + inode = d_inode(chip->bios_dir[i]);
> + inode_lock(inode);
> + inode->i_private = NULL;
> + inode_unlock(inode);
> securityfs_remove(chip->bios_dir[i]);
> + }
> }
> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
> index fd3357e..6df2f8e 100644
> --- a/drivers/char/tpm/tpm_eventlog.h
> +++ b/drivers/char/tpm/tpm_eventlog.h
> @@ -73,7 +73,7 @@ enum tcpa_pc_event_ids {
> HOST_TABLE_OF_DEVICES,
> };
>
> -int read_log(struct tpm_bios_log *log);
> +int read_log(struct tpm_chip *chip);
>
> #if defined(CONFIG_TCG_IBMVTPM) || defined(CONFIG_TCG_IBMVTPM_MODULE) || \
> defined(CONFIG_ACPI)
> diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
> index 570f30c..68d891a 100644
> --- a/drivers/char/tpm/tpm_of.c
> +++ b/drivers/char/tpm/tpm_of.c
> @@ -20,12 +20,14 @@
> #include "tpm.h"
> #include "tpm_eventlog.h"
>
> -int read_log(struct tpm_bios_log *log)
> +int read_log(struct tpm_chip *chip)
> {
> struct device_node *np;
> const u32 *sizep;
> const u64 *basep;
> + struct tpm_bios_log *log;
>
> + log = &chip->log;
> if (log->bios_event_log != NULL) {
> pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
> return -EFAULT;
> --
> 2.5.0
>
^ permalink raw reply
* Re: [PATCH V4 13/15] blk-throttle: add a mechanism to estimate IO latency
From: kbuild test robot @ 2016-11-14 23:40 UTC (permalink / raw)
To: Shaohua Li
Cc: kbuild-all, linux-block, linux-kernel, Kernel-team, axboe, tj,
vgoyal
In-Reply-To: <9c75c9852b08404b90fbbdb143e81a8ef3b36abf.1479161136.git.shli@fb.com>
[-- Attachment #1: Type: text/plain, Size: 2784 bytes --]
Hi Shaohua,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.9-rc5]
[cannot apply to block/for-next next-20161114]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Shaohua-Li/blk-throttle-add-high-limit/20161115-063105
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All warnings (new ones prefixed by >>):
In file included from ./arch/xtensa/include/generated/asm/div64.h:1:0,
from include/linux/kernel.h:142,
from include/linux/list.h:8,
from include/linux/module.h:9,
from block/blk-throttle.c:7:
block/blk-throttle.c: In function 'throtl_calculate_line_slope':
include/asm-generic/div64.h:207:28: warning: comparison of distinct pointer types lacks a cast
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
^
>> block/blk-throttle.c:2017:2: note: in expansion of macro 'do_div'
do_div(xMean, valid_lat);
^
include/asm-generic/div64.h:207:28: warning: comparison of distinct pointer types lacks a cast
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
^
block/blk-throttle.c:2019:2: note: in expansion of macro 'do_div'
do_div(yMean, valid_lat);
^
include/asm-generic/div64.h:207:28: warning: comparison of distinct pointer types lacks a cast
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
^
block/blk-throttle.c:2023:2: note: in expansion of macro 'do_div'
do_div(slope, denominator);
^
vim +/do_div +2017 block/blk-throttle.c
2001 for (i = 0; i < LATENCY_BUCKET_SIZE; i++) {
2002 u64 x, y;
2003
2004 if (td->avg_buckets[i].latency == 0)
2005 continue;
2006
2007 x = td->avg_buckets[i].size >> 10;
2008 y = td->avg_buckets[i].latency;
2009 sumX += x;
2010 sumY += y;
2011
2012 sumXY += x * y;
2013 sumX2 += x * x;
2014 }
2015
2016 xMean = sumX;
> 2017 do_div(xMean, valid_lat);
2018 yMean = sumY;
2019 do_div(yMean, valid_lat);
2020 denominator = sumX2 - sumX * xMean;
2021
2022 slope = sumXY - sumX * yMean;
2023 do_div(slope, denominator);
2024 td->line_slope = slope;
2025 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47027 bytes --]
^ permalink raw reply
* Re: [PATCH 01/12] [media] rc-main: clear rc_map.name in ir_free_table()
From: Shuah Khan @ 2016-11-14 23:41 UTC (permalink / raw)
To: Max Kellermann, linux-media, mchehab; +Cc: linux-kernel, shuah Khan
In-Reply-To: <147077832610.21835.743840405297289081.stgit@woodpecker.blarg.de>
On 08/09/2016 03:32 PM, Max Kellermann wrote:
> rc_unregister_device() will first call ir_free_table(), and later
> device_del(); however, the latter causes a call to rc_dev_uevent(),
> which prints rc_map.name, which at this point has already bee freed.
>
> This fixes a use-after-free bug found with KASAN.
>
> Signed-off-by: Max Kellermann <max.kellermann@gmail.com>
> ---
> drivers/media/rc/rc-main.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index 8e7f292..1e5a520 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -159,6 +159,7 @@ static void ir_free_table(struct rc_map *rc_map)
> {
> rc_map->size = 0;
> kfree(rc_map->name);
> + rc_map->name = NULL;
> kfree(rc_map->scan);
> rc_map->scan = NULL;
> }
>
Hi Mauro,
Could you please get this fix into 4.9. I am seeing the following when I do
rmmod on au0828
[ 179.010878] ==================================================================
[ 179.010895] BUG: KASAN: use-after-free in string+0x170/0x1f0 at addr ffff8801bd513000
[ 179.010900] Read of size 1 by task rmmod/1831
[ 179.010908] CPU: 1 PID: 1831 Comm: rmmod Tainted: G W 4.9.0-rc5 #5
[ 179.010910] Hardware name: Hewlett-Packard HP ProBook 6475b/180F, BIOS 68TTU Ver. F.04 08/03/2012
[ 179.010914] ffff8801aea2f680 ffffffff81b37ad3 ffff8801fa403b80 ffff8801bd513000
[ 179.010922] ffff8801aea2f6a8 ffffffff8156c301 ffff8801aea2f738 ffff8801bd513000
[ 179.010930] ffff8801fa403b80 ffff8801aea2f728 ffffffff8156c59a ffff8801aea2f770
[ 179.010937] Call Trace:
[ 179.010944] [<ffffffff81b37ad3>] dump_stack+0x67/0x94
[ 179.010950] [<ffffffff8156c301>] kasan_object_err+0x21/0x70
[ 179.010954] [<ffffffff8156c59a>] kasan_report_error+0x1fa/0x4d0
[ 179.010968] [<ffffffffa116f05f>] ? au0828_exit+0x10/0x21 [au0828]
[ 179.010973] [<ffffffff8156c8b3>] __asan_report_load1_noabort+0x43/0x50
[ 179.010978] [<ffffffff81b58b20>] ? string+0x170/0x1f0
[ 179.010982] [<ffffffff81b58b20>] string+0x170/0x1f0
[ 179.010987] [<ffffffff81b621c4>] vsnprintf+0x374/0x1c50
[ 179.010992] [<ffffffff81b61e50>] ? pointer+0xa80/0xa80
[ 179.010996] [<ffffffff8156b676>] ? save_stack+0x46/0xd0
[ 179.011001] [<ffffffff81566faa>] ? __kmalloc+0x14a/0x2a0
[ 179.011006] [<ffffffff81b3d70a>] ? kobject_get_path+0x9a/0x200
[ 179.011010] [<ffffffff81b408c2>] ? kobject_uevent_env+0x282/0xca0
[ 179.011014] [<ffffffff81b412eb>] ? kobject_uevent+0xb/0x10
[ 179.011020] [<ffffffff81f10104>] ? device_del+0x434/0x6d0
[ 179.011029] [<ffffffffa0fea717>] ? rc_unregister_device+0x177/0x240 [rc_core]
[ 179.011037] [<ffffffffa116eeb0>] ? au0828_rc_unregister+0x60/0xb0 [au0828]
The problem is fixed with this patch on Linux 4.9-rc4
thanks,
-- Shuah
^ permalink raw reply
* Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels
From: Laurent Pinchart @ 2016-11-14 23:39 UTC (permalink / raw)
To: dri-devel
Cc: Laurent Pinchart, devicetree@vger.kernel.org,
open list:MEDIA DRIVERS FOR RENESAS - FCP, Tomi Valkeinen,
linux-media@vger.kernel.org
In-Reply-To: <1488637.i0jADhlNmg@avalon>
Hi Rob,
Ping ?
On Monday 17 Oct 2016 15:42:56 Laurent Pinchart wrote:
> On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> > On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
> >> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>> Multiple incompatible data link layers have been used over time to
> >>>> transmit image data to LVDS panels. This binding supports display
> >>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>>> specifications.
> >>>>
> >>>> Signed-off-by: Laurent Pinchart
> >>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>> ---
> >>>>
> >>>> .../bindings/display/panel/panel-lvds.txt | 119 ++++++++++++
> >>>> 1 file changed, 119 insertions(+)
> >>>> create mode 100644
> >>>> Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>>>
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> new file mode 100644
> >>>> index 000000000000..250861f2673e
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> @@ -0,0 +1,119 @@
> >>>> +Generic LVDS Panel
> >>>> +==================
> >>>> +
> >>>> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>> Multiple
> >>>> +incompatible data link layers have been used over time to transmit
> >>>> image data
> >>>> +to LVDS panels. This bindings supports display panels compatible with
> >>>> the
> >>>> +following specifications.
> >>>> +
> >>>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>>> February
> >>>> +1999 (Version 1.0), Japan Electronic Industry Development Association
> >>>> (JEIDA)
> >>>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> >>>> National
> >>>> +Semiconductor
> >>>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>>> Video
> >>>> +Electronics Standards Association (VESA)
> >>>> +
> >>>> +Device compatible with those specifications have been marketed under
> >>>> the
> >>>> +FPD-Link and FlatLink brands.
> >>>> +
> >>>> +
> >>>> +Required properties:
> >>>> +- compatible: shall contain "panel-lvds"
> >>>
> >>> Maybe as a fallback, but on its own, no way.
> >>
> >> Which brings an interesting question: when designing generic DT
> >> bindings, what's the rule regarding
>
> Looks like I forgot part of the question. I meant to ask what is the rule
> regarding usage of more precise compatible strings ?
>
> For instance (but perhaps not the best example), the
> input/rotary-encoder.txt bindings define a "rotary-encoder" compatible
> string, with no other bindings defining more precise compatible strings for
> the exact rotary encoder model. When it comes to panels I believe it makes
> sense to define model-specific compatible strings even if they're unused by
> drivers. I'm however wondering what the rule is there, and where those
> device-specific compatible strings should be defined. I'd like to avoid
> using one file per panel model as done today for the simple-panel bindings.
>
> > Call it "simple" so I can easily NAK it. :)
> >
> > Define a generic structure, not a single binding trying to serve all.
> >
> >>> > +- width-mm: panel display width in millimeters
> >>> > +- height-mm: panel display height in millimeters
> >>>
> >>> This is already documented for all panels IIRC.
> >>
> >> Note that this DT binding has nothing to do with the simple-panel
> >> binding. It is instead similar to the panel-dpi and panel-dsi-cm bindings
> >> (which currently don't but should specify the panel size in DT). The LVDS
> >> panel driver will *not* include any panel-specific information such as
> >> size or timings, these are specified in DT.
> >
> > The panel bindings aren't really different. The biggest difference was
> > location in the tree, but we now generally allow panels to be either a
> > child of the LCD controller or connected with OF graph. We probably
> > need to work on restructuring the panel bindings a bit. We should have
> > an inheritance with a base panel binding of things like size, label,
> > graph, backlight, etc, then perhaps an interface specific bindings for
> > LVDS, DSI, and parallel, then a panel specific binding. With this the
> > panel specific binding is typically just a compatible string and which
> > inherited properties apply to it.
>
> That sounds good to me, but we have multiple models for panel bindings.
>
> As you mentioned panels can be referenced through an LCD controller node
> property containing a phandle to the panel node, or through OF graph. That's
> a situation we have today, and we need to keep supporting both (at least
> for existing panels, perhaps not for the new ones).
>
> Another difference is how to express panel data such as size and timings.
> The simple-panel DT bindings don't contain such data and expects the
> drivers to contain a table of panel data for all models supported, while
> the DPI, DSI and now the proposed LVDS panel bindings contain properties
> for panel data.
>
> How would you like to reconcile all that ?
>
> >>>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>>> "jeida-24"
> >>>> + or "vesa-24"
> >>>
> >>> Maybe this should be part of the compatible.
> >>
> >> I've thought about it, but given that some panels support selecting
> >> between multiple modes (through a mode pin that is usually hardwired), I
> >> believe a separate DT property makes sense.
> >
> > Okay.
> >
> >> Furthermore, LVDS data organization is controlled by the combination of
> >> both data-mapping and data-mirror. It makes little sense from my point
> >> of view to handle one as part of the compatible string and the other one
> >> as a separate property.
> >>
> >>> > +Optional properties:
> >>> > +- label: a symbolic name for the panel
> >>>
> >>> Could be for any panel or display connector.
> >>
> >> Yes, but I'm not sure to understand how that's relevant :-)
> >
> > Meaning it should be a common property.
>
> Sure. So you expect me to reorganize all the panels and connectors DT
> bindings in order to get this one merged ? :-)
>
> >>>> +- avdd-supply: reference to the regulator that powers the panel
> >>>> analog supply
> >>>> +- dvdd-supply: reference to the regulator that powers the panel
> >>>> digital supply
> >>>
> >>> Which one has to be powered on first, what voltage, and with what time
> >>> in between? This is why "generic" or "simple" bindings don't work.
> >>
> >> The above-mentioned specifications also define connectors, pinouts and
> >> power supplies, but many LVDS panels compatible with the LVDS physical
> >> and data layers use a different connector with small differences in
> >> power
> >> supplies.
> >>
> >> I believe the voltage is irrelevant here, it doesn't need to be
> >> controlled by the operating system. Power supplies order and timing is
> >> relevant, I'll investigate the level of differences between panels. I'm
> >> also fine with dropping those properties for now.
> >
> > Whether you have control of the supplies is dependent on the board.
> > Dropping them is just puts us in the simple binding trap. The simple
> > bindings start out that way and then people keep adding to them.
>
> Damn, you can't be fooled easily ;-)
>
> On a more serious note, I'd like to design the bindings in a way that
> wouldn't require adding device-specific code in the driver for each panel
> model, given that in most cases power supply handling will be generic.
> What's your opinion about a generic power supply model that would be used
> in the default case, with the option to override it with device-specific
> code when needed ?
>
> >>>> +- data-mirror: if set, reverse the bit order on all data lanes (6 to
> >>>> 0 instead
> >>>> + of 0 to 6)
> >
> > On this one, make the name describe the order. "mirror" requires that
> > I know what is normal ordering. Perhaps "data-msb-first".
>
> Sounds good to me, I'll use that.
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels
From: Laurent Pinchart @ 2016-11-14 23:39 UTC (permalink / raw)
To: dri-devel
Cc: Rob Herring, devicetree@vger.kernel.org, Laurent Pinchart,
open list:MEDIA DRIVERS FOR RENESAS - FCP, Tomi Valkeinen,
linux-media@vger.kernel.org
In-Reply-To: <1488637.i0jADhlNmg@avalon>
Hi Rob,
Ping ?
On Monday 17 Oct 2016 15:42:56 Laurent Pinchart wrote:
> On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> > On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
> >> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>> Multiple incompatible data link layers have been used over time to
> >>>> transmit image data to LVDS panels. This binding supports display
> >>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>>> specifications.
> >>>>
> >>>> Signed-off-by: Laurent Pinchart
> >>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>> ---
> >>>>
> >>>> .../bindings/display/panel/panel-lvds.txt | 119 ++++++++++++
> >>>> 1 file changed, 119 insertions(+)
> >>>> create mode 100644
> >>>> Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>>>
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> new file mode 100644
> >>>> index 000000000000..250861f2673e
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> @@ -0,0 +1,119 @@
> >>>> +Generic LVDS Panel
> >>>> +==================
> >>>> +
> >>>> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>> Multiple
> >>>> +incompatible data link layers have been used over time to transmit
> >>>> image data
> >>>> +to LVDS panels. This bindings supports display panels compatible with
> >>>> the
> >>>> +following specifications.
> >>>> +
> >>>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>>> February
> >>>> +1999 (Version 1.0), Japan Electronic Industry Development Association
> >>>> (JEIDA)
> >>>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> >>>> National
> >>>> +Semiconductor
> >>>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>>> Video
> >>>> +Electronics Standards Association (VESA)
> >>>> +
> >>>> +Device compatible with those specifications have been marketed under
> >>>> the
> >>>> +FPD-Link and FlatLink brands.
> >>>> +
> >>>> +
> >>>> +Required properties:
> >>>> +- compatible: shall contain "panel-lvds"
> >>>
> >>> Maybe as a fallback, but on its own, no way.
> >>
> >> Which brings an interesting question: when designing generic DT
> >> bindings, what's the rule regarding
>
> Looks like I forgot part of the question. I meant to ask what is the rule
> regarding usage of more precise compatible strings ?
>
> For instance (but perhaps not the best example), the
> input/rotary-encoder.txt bindings define a "rotary-encoder" compatible
> string, with no other bindings defining more precise compatible strings for
> the exact rotary encoder model. When it comes to panels I believe it makes
> sense to define model-specific compatible strings even if they're unused by
> drivers. I'm however wondering what the rule is there, and where those
> device-specific compatible strings should be defined. I'd like to avoid
> using one file per panel model as done today for the simple-panel bindings.
>
> > Call it "simple" so I can easily NAK it. :)
> >
> > Define a generic structure, not a single binding trying to serve all.
> >
> >>> > +- width-mm: panel display width in millimeters
> >>> > +- height-mm: panel display height in millimeters
> >>>
> >>> This is already documented for all panels IIRC.
> >>
> >> Note that this DT binding has nothing to do with the simple-panel
> >> binding. It is instead similar to the panel-dpi and panel-dsi-cm bindings
> >> (which currently don't but should specify the panel size in DT). The LVDS
> >> panel driver will *not* include any panel-specific information such as
> >> size or timings, these are specified in DT.
> >
> > The panel bindings aren't really different. The biggest difference was
> > location in the tree, but we now generally allow panels to be either a
> > child of the LCD controller or connected with OF graph. We probably
> > need to work on restructuring the panel bindings a bit. We should have
> > an inheritance with a base panel binding of things like size, label,
> > graph, backlight, etc, then perhaps an interface specific bindings for
> > LVDS, DSI, and parallel, then a panel specific binding. With this the
> > panel specific binding is typically just a compatible string and which
> > inherited properties apply to it.
>
> That sounds good to me, but we have multiple models for panel bindings.
>
> As you mentioned panels can be referenced through an LCD controller node
> property containing a phandle to the panel node, or through OF graph. That's
> a situation we have today, and we need to keep supporting both (at least
> for existing panels, perhaps not for the new ones).
>
> Another difference is how to express panel data such as size and timings.
> The simple-panel DT bindings don't contain such data and expects the
> drivers to contain a table of panel data for all models supported, while
> the DPI, DSI and now the proposed LVDS panel bindings contain properties
> for panel data.
>
> How would you like to reconcile all that ?
>
> >>>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>>> "jeida-24"
> >>>> + or "vesa-24"
> >>>
> >>> Maybe this should be part of the compatible.
> >>
> >> I've thought about it, but given that some panels support selecting
> >> between multiple modes (through a mode pin that is usually hardwired), I
> >> believe a separate DT property makes sense.
> >
> > Okay.
> >
> >> Furthermore, LVDS data organization is controlled by the combination of
> >> both data-mapping and data-mirror. It makes little sense from my point
> >> of view to handle one as part of the compatible string and the other one
> >> as a separate property.
> >>
> >>> > +Optional properties:
> >>> > +- label: a symbolic name for the panel
> >>>
> >>> Could be for any panel or display connector.
> >>
> >> Yes, but I'm not sure to understand how that's relevant :-)
> >
> > Meaning it should be a common property.
>
> Sure. So you expect me to reorganize all the panels and connectors DT
> bindings in order to get this one merged ? :-)
>
> >>>> +- avdd-supply: reference to the regulator that powers the panel
> >>>> analog supply
> >>>> +- dvdd-supply: reference to the regulator that powers the panel
> >>>> digital supply
> >>>
> >>> Which one has to be powered on first, what voltage, and with what time
> >>> in between? This is why "generic" or "simple" bindings don't work.
> >>
> >> The above-mentioned specifications also define connectors, pinouts and
> >> power supplies, but many LVDS panels compatible with the LVDS physical
> >> and data layers use a different connector with small differences in
> >> power
> >> supplies.
> >>
> >> I believe the voltage is irrelevant here, it doesn't need to be
> >> controlled by the operating system. Power supplies order and timing is
> >> relevant, I'll investigate the level of differences between panels. I'm
> >> also fine with dropping those properties for now.
> >
> > Whether you have control of the supplies is dependent on the board.
> > Dropping them is just puts us in the simple binding trap. The simple
> > bindings start out that way and then people keep adding to them.
>
> Damn, you can't be fooled easily ;-)
>
> On a more serious note, I'd like to design the bindings in a way that
> wouldn't require adding device-specific code in the driver for each panel
> model, given that in most cases power supply handling will be generic.
> What's your opinion about a generic power supply model that would be used
> in the default case, with the option to override it with device-specific
> code when needed ?
>
> >>>> +- data-mirror: if set, reverse the bit order on all data lanes (6 to
> >>>> 0 instead
> >>>> + of 0 to 6)
> >
> > On this one, make the name describe the order. "mirror" requires that
> > I know what is normal ordering. Perhaps "data-msb-first".
>
> Sounds good to me, I'll use that.
--
Regards,
Laurent Pinchart
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.